Fix error in EventHandler on request with error #1114
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a request has an error, it would give the `on_finish` callback a response object that is `nil`. This would break when we called `response.status`, and it would fail to complete the transaction. Make sure the transaction is always completed when it's handling a transaction, by putting the outside the safety block which fetches all the request and response metadata. Check if the `response` object is `nil` before calling other methods on it to the response status tag and metric. This way we don't raise an error. Luckily no integrations of ours that have been published yet would run into this behavior. The Rails integrations uses the EventHandler, but has its own error reporting. This would only be a problem for people who use the EventHandler directly, or when we move more integrations over to use AbstractMiddleware subclasses.
tombruijn
force-pushed
the
rack-eventhandler-errors
branch
from
7f6c1e9 to
9e6a3d4
Compare
luismiramirez
approved these changes
Jun 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a request has an error, it would give the
on_finishcallback a response object that isnil.This would break when we called
response.status, and it would fail to complete the transaction.Make sure the transaction is always completed by putting the
complete_current!call inensureblocks.Check if the
responseobject isnilbefore calling other methods on it to the response status tag and metric. This way we don't raise an error.Luckily no integrations of ours that have been published yet would run into this behavior. The Rails integrations uses the EventHandler, but has its own error reporting.
This would only be a problem for people who use the EventHandler directly, or when we move more integrations over to use AbstractMiddleware subclasses.