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

[BUG] Events created in nested transactions can fail to deliver #452

Closed
3 tasks done
ryanshellberg opened this issue May 30, 2024 · 2 comments
Closed
3 tasks done

Comments

@ryanshellberg
Copy link

Bug Report

Describe the Bug:
Originally called out as a concern in the V2 PR. When an event is created inside of a transaction, the corresponding EventJob fires before the event itself is created. This leads to a DeserializationError which is then discarded, so the failure was quite tricky to track down.

To Reproduce:
A little tricky to reproduce here because it is a race condition, but our code looks like this:

Person.transaction do 
  ... other changes ...
  ReportComplete.with(report: report).deliver(report.user)
end

Expected Behavior:
EventJob's are created after the event is committed to the database.

Actual Behavior:
EventJob's are fired off before the event is committed to the database.

Environment:

  • Noticed gem version: 2.3.1
  • Ruby version: 3.3.1
  • Rails version: 7.1.3.3
  • Operating System: OSX 14.5

Possible Fix:
Seems like using an after_commit callback to fire the job should work.

Related Issues:
#313 (comment)

Checklist:

  • I have searched for similar issues and couldn't find any
  • I have checked the documentation for relevant information
  • I have included all the required information
@excid3
Copy link
Owner

excid3 commented May 30, 2024

I wouldn't consider this a bug in Noticed. You should deliver the notification after your transaction is completed.

By wrapping Noticed in another transaction, it negates the transaction Noticed uses internally.

Rails 7.2 is going to help with this by enqueuing after transaction: https://edgeguides.rubyonrails.org/7_2_release_notes.html#prevent-jobs-from-being-scheduled-within-transactions

@ryanshellberg
Copy link
Author

Good point, thanks for the info on the upcoming Rails change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants