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

Change transition failed error to explicitly include to / from states #367

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

ahjmorton
Copy link
Contributor

@ahjmorton ahjmorton commented Nov 6, 2019

Make the from / to states in the TransitionFailedError accessible to avoid parsing strings to determine which state transition failed.

Allows the recipient to make more informed decisions when a state transition fails. For example if we're trying to transition to the same state due to a race condition we should take a different action than if we're transitioning to something that is invalid as a result of a programming error.

Maintain backwards compatibility with those who parsed the exception to get the transition states by maintaining the same error message.

Copy link
Contributor

@danwakefield danwakefield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. If you are going to bump version now you will need to update the changelog with the other recent minor changes e.g after_transition_failure

@ahjmorton ahjmorton force-pushed the add-explicit-parameters-to-exception branch from 011e0c5 to c672e7b Compare November 6, 2019 12:02
@ahjmorton
Copy link
Contributor Author

ahjmorton commented Nov 6, 2019

@danwakefield for now I've removed the versioning commit. Will re-add it on a tagged branch to release off of

@ahjmorton ahjmorton merged commit b7e1a9c into master Nov 6, 2019
@ahjmorton ahjmorton deleted the add-explicit-parameters-to-exception branch November 6, 2019 12:05
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