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

Improved Exceptions in TransactionManager #862

Merged
merged 2 commits into from
Sep 15, 2020
Merged

Improved Exceptions in TransactionManager #862

merged 2 commits into from
Sep 15, 2020

Conversation

jwilger
Copy link
Contributor

@jwilger jwilger commented Sep 2, 2020

The use of generic RuntimeError exceptions coming from the TransactionManager requires brittle error-handling in dependant code, so I've changed most of the raise statements in the TransactionManager to use the Kafka::InvalidTxnStateError exception.

Additionally, there may be cases where dependant code starts a transaction using the producer's #transaction block syntax and then an exception is raised within that block before the transaction manager actually opens up a transaction. In such cases, prior to this change, the real exception is obfuscated, because the transaction manager would raise an exception
about not being in a valid transactional state.

With this change, attempting to abort a transaction that has not actually been opened will no longer result in an exception. Instead it will log a warning message and leave the transaction manager in its ready state.

The use of generic RuntimeError exceptions requires brittle
error-handling in dependant code.
There may be cases where dependant code starts a transaction using the
producer's `#transaction` block syntax and then an exception is raised
within that block *before* the transaction manager actually opens up a
transaction. In such cases, prior to this change, the real exception is
obfuscated, because the transaction manager would raise an exception
about not being in a valid transactional state.

With this change, attempting to abort a transaction that has not
actually been opened will no longer result in an exception. Instead it
will log a warning message and leave the transaction manager in its
ready state.
@dasch dasch merged commit f248f59 into zendesk:master Sep 15, 2020
@dasch
Copy link
Contributor

dasch commented Sep 15, 2020

Thanks!

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.

2 participants