Skip to content

SQS: Properly handle Errors in listeners#1383

Merged
tomazfernandes merged 1 commit into
awspring:mainfrom
isaacvando:main
May 18, 2025
Merged

SQS: Properly handle Errors in listeners#1383
tomazfernandes merged 1 commit into
awspring:mainfrom
isaacvando:main

Conversation

@isaacvando
Copy link
Copy Markdown
Contributor

@isaacvando isaacvando commented May 8, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

I updated the listener adapters to catch Throwable instead of Exception so that ErrorHandlers can handle Throwables as expected.

💡 Motivation and Context

Fixes #1379.

💚 How did you test it?

I added unit tests to the classes I modified and tested the change locally within my own project against the reproduction in the issue.

I would be happy to add an integration test if appropriate. If so could you give me some direction about your preferences for that?

📝 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

@github-actions github-actions Bot added the component: sqs SQS integration related issue label May 8, 2025
@isaacvando isaacvando marked this pull request as ready for review May 8, 2025 20:57
@tomazfernandes
Copy link
Copy Markdown
Contributor

Hey @isaacvando, thanks for the PR!

I would be happy to add an integration test if appropriate. If so could you give me some direction about your preferences for that?

I'm happy with the tests you added, so unless you'd like to test an specific flow with an integration test, I don't think we'd need specific tests to assert correct handling for Error. If you do think there's a flow you'd like to test though, feel free to.

On a quick search on the codebase for catch(Exception I saw there's quite a few other places in SQS (and other integrations) where we have this problem, so if you would like to pick up a few more on SQS side that would be really helpful.

While Errors are rarely thrown, I've recently had just that situation with an AWS Glue deserializer and lost a few precious hours to understand what the issue was, so if we can avoid our users from going through that pain, it'd be better.

Totally up to you though, happy to merge this PR as is and open a new issue for that.

Please let me know your thoughts, thanks.

@isaacvando
Copy link
Copy Markdown
Contributor Author

Great, @tomazfernandes! I will be out of office for a while so I think it would be best to go ahead and merge the PR now. Thanks!

@tomazfernandes tomazfernandes merged commit ad64664 into awspring:main May 18, 2025
5 checks passed
@tomazfernandes
Copy link
Copy Markdown
Contributor

Thanks for the PR @isaacvando, looking forward to more!

I've opened the issue for the remaining Exceptions:
#1386

kcsurapaneni pushed a commit to kcsurapaneni/spring-cloud-aws that referenced this pull request Jun 13, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errors are not handled when thrown in an SQS listener

2 participants