-
Notifications
You must be signed in to change notification settings - Fork 26
fix: Fix Railtie middleware insertion crashing on Rails initialization #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fix Railtie middleware insertion crashing on Rails initialization #98
Conversation
The initializer block called insert_middleware_after as a class method, but Rails runs initializer blocks via instance_exec on the Railtie instance. Also removed the include? check on MiddlewareStackProxy which doesn't support query methods during initialization.
When a message exceeds the 32KB size limit, it is silently dropped from the batch. Previously the worker would still send the empty batch to the API, which returned a non-JSON response, causing a JSON::ParserError that triggered 10 pointless retries. - Skip transport request when the batch is empty after consuming the queue - Rescue JSON::ParserError in Transport#send and fall back to the raw response body instead of crashing
The CaptureExceptions middleware was setting $request_url, which is not recognized by PostHog for the "URL / Screen" column. Renamed to $current_url to match the standard property used across PostHog SDKs.
Exception payloads frequently exceeded the 32KB message limit because context lines (11 lines of source code) were read for every stack frame, including gem and framework frames. A typical Rails backtrace has ~50 frames, most from Rails internals, easily blowing past the limit and causing the event to be silently dropped. Now context lines are only added for in_app frames. Gem/framework frames still include file, line number, and function name. Fixes PostHog#88
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @johnnagro , those all are really good fixes! Do you mind adding them to the CHANGELOG so that we can release them? Also, do you mind bumping PostHog::VERSION by one patch version?
We'll be improving this slightly in the future but, for now, that's how we're handling this. Or you can give me access to your repo and I can do it myself
Also, rubocop is complaining
|
@rafaeelaudibert i'm happy to help. I've made the changes you've requested. At your convenience please take another look and let me know if you'd like me to make any additional tweaks. |
| # Fallback: append to stack if target middleware is missing (e.g., API-only apps) | ||
| app.config.middleware.use(middleware) | ||
| end | ||
| def insert_middleware_after(app, target, middleware) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a problem here, though. If target doesn't exist then we won't insert our middleware, and that's a bad failure. I wonder if there's a better way. Will keep it like this for now, but I'll keep thinking about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed - i opened #99 and we can think through a more robust approach
|
@rafaeelaudibert i dont know if the |
|
@johnnagro nah, you don't need to worry about that one, it's broken for a different reason - it'll always be broken for external contributors because of the permissions we have in some repos. Thank you for this fix! |
fixes #97, #87, #88
Fixes initializer middleware errors #97
The initializer block called insert_middleware_after as a class method, but Rails runs initializer blocks via instance_exec on the Railtie instance. Also removed the include? check on MiddlewareStackProxy which doesn't support query methods during initialization.
I created a fresh rails project, added
posthog-rubyand the local copy of thisposthog-railsbranch. Then verified thatbin/rails generate posthog:installruns cleanly again.Fixes Exception capture payload & Empty Batch Errors #87
Also fixes when a message exceeds the 32KB size limit, it is silently dropped from the batch. Previously the worker would still send the empty batch to the API, which returned a non-JSON response, causing a JSON::ParserError
that triggered 10 pointless retries.
response body instead of crashing
Only include source context lines for in-app exception frames #88
Exception payloads frequently exceeded the 32KB message limit because
context lines (11 lines of source code) were read for every stack frame,
including gem and framework frames. A typical Rails backtrace has ~50
frames, most from Rails internals, easily blowing past the limit and
causing the event to be silently dropped.
Now context lines are only added for in_app frames. Gem/framework frames
still include file, line number, and function name.
Fixes Use $current_url property so exception URL appears in PostHog UI
The CaptureExceptions middleware was setting $request_url, which is not
recognized by PostHog for the "URL / Screen" column. Renamed to
$current_url to match the standard property used across PostHog SDKs.