-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: add error message parameter to onError
callback in subscription
#2987
feat: add error message parameter to onError
callback in subscription
#2987
Conversation
onError
callback in subscriptiononError
callback in subscription
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2987 +/- ##
=======================================
Coverage 92.57% 92.57%
=======================================
Files 85 85
Lines 3166 3166
Branches 776 776
=======================================
Hits 2931 2931
Misses 183 183
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, this is a nice enhancement, but, isn't it automatically expose some unwanted error messages to the client as well? For instance, when an endpoint method returning a Signal is annotated with @RolesAllowed("ADMIN")
, so far, the only thing on client was: "Error in flux ...", and I wonder if this change exposes the the "access denied" error message to the client, and cause a security issue.
…llback-of-Subscription
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just adding this to block merging before the above question is answered.
The messages we're talking about here are already on the client. Without a callback, an error is thrown with the contents of the message. With a callback, the message was lost. If the message contents are an issue on their own, I think that a separate issue should be opened. |
…llback-of-Subscription
Quality Gate passedIssues Measures |
This ticket/PR has been released with Hilla 24.7.0.alpha3 and is also targeting the upcoming stable 24.7.0 version. |
Closes #2062.