Add Annotation SentryCaptureException for org.springframework.web.bind.annotation.ExceptionHandler

[xeromank]

📜 Description

Add Annotation SentryCaptureException for org.springframework.web.bind.annotation.ExceptionHandler

💡 Motivation and Context

It was inconvenient to use Sentry.captureException(Exception e) in the @ExceptionHandler of @ControllerAdvice, so that part was annotated. In the process, since the company does not have a private repository, a Pull Request is made.

💚 How did you test it?

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
• I updated the docs if needed.
• Review from the native team if needed.
• No breaking change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[adinauer]

Thanks for the PR @xeromank. Will try to give this a review soon.

[adinauer]

I've just given this a pass, fixed some config and added tests. Before duplicating this into the non jakarta modules for Spring 5 and Spring Boot 2 I'd like to discuss some things internally:

• Should we capture Throwable instead?
• Do we want an option to opt-in / opt-out?
• Should we allow annotating classes as well? Probably not.
• Is handled = true what we want? Should it be a property on the annotation to allow configuring it?

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
...y/spring/boot/jakarta/SentryAutoConfiguration.java	97.33% <100.00%> (+0.07%)	⬆️
...io/sentry/spring/boot/SentryAutoConfiguration.java	97.33% <100.00%> (+0.07%)	⬆️
.../SentryCaptureExceptionParameterConfiguration.java	100.00% <100.00%> (ø)
...aptureExceptionParameterPointcutConfiguration.java	100.00% <100.00%> (ø)
...n/SentryExceptionParameterAdviceConfiguration.java	100.00% <100.00%> (ø)
.../SentryCaptureExceptionParameterConfiguration.java	100.00% <100.00%> (ø)
...aptureExceptionParameterPointcutConfiguration.java	100.00% <100.00%> (ø)
...n/SentryExceptionParameterAdviceConfiguration.java	100.00% <100.00%> (ø)
...ception/SentryCaptureExceptionParameterAdvice.java	90.47% <90.47%> (ø)
...ception/SentryCaptureExceptionParameterAdvice.java	90.47% <90.47%> (ø)

📢 Thoughts on this report? Let us know!.

[adinauer]

Should we capture Throwable instead?

Yes

Do we want an option to opt-in / opt-out?

Opt-out as this may be causing issues GraalVM (see #2742)

Should we allow annotating classes as well? Probably not.

Not for now. Can always come back later and revisit.

Is handled = true what we want? Should it be a property on the annotation to allow configuring it?

Not for now. Can always come back later and revisit.

---

Next step would be to copy changes over to spring (non jakarta) modules.

[xeromank]

• Should we capture Throwable instead?

Yes

• Do we want an option to opt-in / opt-out?

No. unnecessary

• Should we allow annotating classes as well? Probably not.

No. as you expected

• Is handled = true what we want? Should it be a property on the annotation to allow configuring it?

To be honest, I'm not sure about this part. But I think this will be convenient.

[adinauer]

@xeromank I'll take another look here but want to wait for #2788 to get merged first, resolve conflicts and only then start copying code.

[xeromank]

I checked PR(#2946). Thanks!!
I will close this PR.

[adinauer]

Hello @xeromank the @SentryCheckIn annotation is for check-ins (CRONS) where the goal is to monitor a scheduled job to see if it executes on schedule and also provide a trace for it. Are you sure it replaces this PR?

[xeromank]

Hello @adinauer. thanks for explaining.
I thought it seemed to have a somewhat similar effect.
@SentryCheckIn can't play the role of Sentry.capture(), right?

[adinauer]

@xeromank well it does capture, but not errors. It captures a check-in for a CRON job. Here's some docs on the product: https://docs.sentry.io/product/crons/

I'm thinking whether we wanna reserve the name @SentryCaptureException for an annotation that works a bit differently from what the one in this PR does. I was thinking it could basically wrap the method in a try/catch and capture any exceptions the method throws. The annotation in this PR could be named @SentryCaptureExceptionParam or similar to make it more clear it captures an exception passed in to the annotated method vs. one that's thrown. What do you think?

[xeromank]

@adinauer
I will respect your opinion.
The name of the annotation is not important to me.
I just wish I was good at capturing exceptions.

[adinauer]

Will give this another pass and change some things later this week hopefully.

[xeromank]

@adinauer thank you. I'll be keeping an eye on the release history.

[adinauer]

Changed naming of the annotation to @SentryCaptureExceptionParameter, moved it into a different package, added an opt-out flag and duplicated changes into Spring Boot 2. Thanks again for the PR @xeromank