Skip to content
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 error in AS when transaction get completed #532

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

tombruijn
Copy link
Member

@tombruijn tombruijn commented Jun 3, 2019

If an AppSignal transaction is closed in an instrumentation block (which
shouldn't happen), the transaction can no longer be called to finish an
event.

ActiveSupport::Notifications.instrument("foo") do
  Appsignal::Transaction.complete_current!
end

To fix the error that would occur, fetch the transaction again from
Appsignal::Transaction.current instead of using the transaction
variable that was available. If the transaction was closed inside the
instrumentation block, it will return a NilTransaction which will
ignore the method call instead of raising an error.

If an AppSignal transaction is closed in an instrumentation block (which
shouldn't happen), the transaction can no longer be called to finish an
event.

```
ActiveSupport::Notifications.instrument("foo") do
  Appsignal::Transaction.complete_current!
end
```

To fix the error that would occur, fetch the transaction again from
`Appsignal::Transaction.current` instead of using the `transaction`
variable that was available. If the transaction was closed inside the
instrumentation block, it will return a `NilTransaction` which will
ignore the method call instead of raising an error.
@tombruijn tombruijn added the bug label Jun 3, 2019
@tombruijn tombruijn self-assigned this Jun 3, 2019
@tombruijn
Copy link
Member Author

@tombruijn tombruijn merged commit dea4e18 into master Jun 4, 2019
@tombruijn tombruijn deleted the activesupport-event-finish branch June 4, 2019 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants