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

Verify behaviour of an AfterCommitAsyncDispatcher when there was raise in some after_commit callback #519

Merged
merged 1 commit into from
Dec 19, 2018

Conversation

mostlyobvious
Copy link
Member

@mostlyobvious mostlyobvious commented Dec 19, 2018

It is possible that after_commit callback in one of ActiveRecords has raised. In such case:

  • error is bubbled up after transaction block
  • commited! is called with additional argument (with specifics
    depending on Rails version)

What it means for us is that:

  • transaction is commited, records are saved (and events appended)
    thus proceeding with scheduling of handling the event
  • AsyncRecord#committed must accept additional arguments to not fail
    scheduling in this case

Closes: #183

It is possible that after_commit in one of ActiveRecords has failed
(raised). In such case:

  * error is bubbled up after transaction block
  * commited! is called with additional argument (with specifics
    depending on Rails version)

What it means for us is that:

  * transaction is commited, records are saved (and events appended)
    thus proceeding with scheduling of handling the event
  * AsyncRecord#committed must accept additional arguments to not fail
    scheduling in this case

[#183]
@mostlyobvious mostlyobvious changed the title Verify behaviour of an AfterCommitAsyncDispatcher when there was raise in some after_commiit callback Verify behaviour of an AfterCommitAsyncDispatcher when there was raise in some after_commit callback Dec 19, 2018
@mostlyobvious mostlyobvious merged commit 48595e1 into master Dec 19, 2018
@mostlyobvious mostlyobvious deleted the bug-183 branch December 19, 2018 18:24
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

Successfully merging this pull request may close these issues.

1 participant