Skip to content

ImmediateRetryAsyncErrorHandler changes msg visibility timeout to zero. (#1314)#1370

Merged
tomazfernandes merged 1 commit into
awspring:mainfrom
brun0-4ugusto:async-error-handler
May 3, 2025
Merged

ImmediateRetryAsyncErrorHandler changes msg visibility timeout to zero. (#1314)#1370
tomazfernandes merged 1 commit into
awspring:mainfrom
brun0-4ugusto:async-error-handler

Conversation

@rafaelcgpava
Copy link
Copy Markdown
Contributor

@rafaelcgpava rafaelcgpava commented Apr 11, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

New ImmediateRetryAsyncErrorHandler to change message visibility timeout to zero when an error occurs

💡 Motivation and Context

As mentioned in (#1313), it was not possible to immediatly report back an error to AWS SQS. This enhancement allows this feature to immediatly make the message available for retry by changing it's visibility timeout to zero.

See #1314

💚 How did you test it?

Unit test, Integration test, manual tests with a sample application

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated reference documentation to reflect the change
  • All tests passing
  • No breaking changes

🔮 Next steps

  • Update sample application
  • Develop ImmediateRetryErrorHandler

@github-actions github-actions Bot added the component: sqs SQS integration related issue label Apr 11, 2025
Copy link
Copy Markdown
Contributor

@tomazfernandes tomazfernandes left a comment

Choose a reason for hiding this comment

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

Hey Bruno and Rafael, thanks for the PR!

I left a few comments, please let me know if they make sense to you or if you have any questions.

Thanks!

@rafaelcgpava
Copy link
Copy Markdown
Contributor Author

Hey Bruno and Rafael, thanks for the PR!

I left a few comments, please let me know if they make sense to you or if you have any questions.

Thanks!

Hey there Tomaz! Thank you for reviewing our PR.

Your suggestions make a lot of sense and we have just commited the changes.

Please let us know if there is anything else we could change/improve.

@rafaelcgpava rafaelcgpava changed the title AsyncDefaultErrorHandler changes msg visibility timeout to zero. (#1314) ImmediateRetryAsyncErrorHandler changes msg visibility timeout to zero. (#1314) Apr 11, 2025
Copy link
Copy Markdown
Contributor

@tomazfernandes tomazfernandes left a comment

Choose a reason for hiding this comment

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

Hey @rafaelcgpava, thanks for the adjustments!

This looks good, I've just left a couple more details for your consideration.

I'll also ask you to add this feature to the docs (sqs.adoc), including how to set it up (just declare it as a bean for auto-configuration, or add to a Factory / Container directly).

Please let me know if you have any questions / suggestions.

@github-actions github-actions Bot added the type: documentation Documentation or Samples related issue label Apr 18, 2025
@brun0-4ugusto
Copy link
Copy Markdown
Contributor

Hey @tomazfernandes, thank you for the review!

We've just pushed the requested changes. Please let us know if there's anything else we should revise or improve. 🎉 🚀

Copy link
Copy Markdown
Contributor

@tomazfernandes tomazfernandes left a comment

Choose a reason for hiding this comment

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

Almost there @rafaelcgpava and @brun0-4ugusto!

Comment thread docs/src/main/asciidoc/sqs.adoc Outdated
Comment thread docs/src/main/asciidoc/sqs.adoc Outdated
Comment thread docs/src/main/asciidoc/sqs.adoc Outdated
@rafaelcgpava
Copy link
Copy Markdown
Contributor Author

rafaelcgpava commented Apr 18, 2025

@tomazfernandes once again, thank you very much for your feedback and the suggestions you have been making! It has been a great learning experience!

I´ve just commited the changes we've made from your suggestions =)

I had a question about the note we´ve left saying that there was only the AsyncErrorHandler implementation for ImmediateRetry, however I checked that we adapt ErrorHandlers to AsyncErrorHandlers using BlockingErrorHandlerAdapter, and it makes a lot of sense to remove that note.

@brun0-4ugusto
Copy link
Copy Markdown
Contributor

Hi @tomazfernandes! I hope this message finds you well. I just wanted to kindly check if there are any changes needed on the PR — I’m available to assist with any updates you might require.

Copy link
Copy Markdown
Contributor

@tomazfernandes tomazfernandes left a comment

Choose a reason for hiding this comment

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

Looking great @rafaelcgpava and @brun0-4ugusto , thanks for the PR!

Looking forward to new PRs!

If you want a suggestion, how about we create a LinearBackOffErrorHandler, or an ExponentialBackOffErrorHandler?

Your call, please let me know if there's anything I can help with.

Thanks again!

@tomazfernandes tomazfernandes merged commit 2e14170 into awspring:main May 3, 2025
5 checks passed
@brun0-4ugusto
Copy link
Copy Markdown
Contributor

Thanks again, @tomazfernandes! We’ve already started working on the ExponentialBackOffErrorHandler. I believe we’ll submit the PR this week. 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: sqs SQS integration related issue type: documentation Documentation or Samples related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants