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(idempotency): include cause in idempotency persistence layer error #2916

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

dreamorosi
Copy link
Contributor

Summary

Changes

Please provide a summary of what's being changed

This PR removes the redeclared cause property from the IdempotencyPersistenceLayerError class, so that it can inherit the value from its parent.

This will solve the bug described in the linked issue, additionally I also updated some of the unit tests to make sure the cause is defined.

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: closes #2915


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi requested review from a team as code owners August 12, 2024 16:29
@boring-cyborg boring-cyborg bot added idempotency This item relates to the Idempotency Utility tests PRs that add or change tests labels Aug 12, 2024
@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Aug 12, 2024
@github-actions github-actions bot added the bug Something isn't working label Aug 12, 2024
@dreamorosi dreamorosi added the do-not-merge This item should not be merged label Aug 12, 2024
@dreamorosi dreamorosi self-assigned this Aug 12, 2024
@dreamorosi
Copy link
Contributor Author

Quality Gate Failed Quality Gate failed

Failed conditions 68.2% Duplication on New Code (required ≤ 50%)

See analysis details on SonarCloud

This is addressed in #2919, this PR should be merged after that one is merged.

@dreamorosi dreamorosi force-pushed the fix/idempotency_persistence_error_cause branch from d4f18e7 to bd318c5 Compare August 13, 2024 17:50
@dreamorosi dreamorosi removed the do-not-merge This item should not be merged label Aug 13, 2024
@dreamorosi dreamorosi requested a review from am29d August 13, 2024 17:55
Copy link
Contributor

@am29d am29d left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

Copy link

@am29d am29d merged commit 47f0161 into main Aug 13, 2024
9 checks passed
@am29d am29d deleted the fix/idempotency_persistence_error_cause branch August 13, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working idempotency This item relates to the Idempotency Utility size/M PR between 30-99 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: IdempotencyPersistenceLayerError cause undefined
2 participants