ref(sentry apps): Improve Sentry App Component endpoint errors #81167
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #81167 +/- ##
==========================================
- Coverage 80.45% 80.37% -0.09%
==========================================
Files 7241 7224 -17
Lines 321509 319645 -1864
Branches 20810 20755 -55
==========================================
- Hits 258674 256907 -1767
+ Misses 62436 62345 -91
+ Partials 399 393 -6 |
… errors (#81480) In (#81167 ), we want to change the SentryAppComponent endpoint to return a string of the error to improve end user clarity when debugging. In the frontend, we currently expect the error response to be a boolean though, so we need to make the frontend backwards compatible (i.e handle both boolean and string)
| @@ -14,7 +14,7 @@ def serialize(self, obj, attrs, user, **kwargs): | |||
| "uuid": str(obj.uuid), | |||
| "type": obj.type, | |||
| "schema": obj.schema, | |||
| "error": True if str(obj.uuid) in errors else False, | |||
| "error": errors.get(str(obj.uuid), ""), | |||
There was a problem hiding this comment.
In the above we catch all errors (assuming they're raised), so there should never be the case where a component in in errors but the message is an empty string.
There was a problem hiding this comment.
what does it look like on the frontend when we return the entire error instead of a bool?
There was a problem hiding this comment.
Nothing will change for frontend appearance wise. Currently we use that boolean to determine whether or not we need to gray out the UI component and display a generic 'couldn't connect to [SentryApp]' message. That message and appearance will stay the same, the new errors will be exposed via the network requests.
There was a problem hiding this comment.
should that be a change we make -- to surface the more explicit error message?
There was a problem hiding this comment.
we could, but that would be a future PR. Our integrators are all developers who will check the network reqs to debug cust issues, so this change is needed first to provide that visibility.
| @@ -14,7 +14,7 @@ def serialize(self, obj, attrs, user, **kwargs): | |||
| "uuid": str(obj.uuid), | |||
| "type": obj.type, | |||
| "schema": obj.schema, | |||
| "error": True if str(obj.uuid) in errors else False, | |||
| "error": errors.get(str(obj.uuid), ""), | |||
There was a problem hiding this comment.
should that be a change we make -- to surface the more explicit error message?
ney this is a private endpoint |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
… errors (#81480) In (#81167 ), we want to change the SentryAppComponent endpoint to return a string of the error to improve end user clarity when debugging. In the frontend, we currently expect the error response to be a boolean though, so we need to make the frontend backwards compatible (i.e handle both boolean and string)
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
tl;dr Instead of returning a json with
error: Truereturn a json witherror: "ValidationError: bruh"Expose sentry app component endpoint errors. Instead of returning a boolean indicating that an error occurred while trying to make the component we return the error message.
Context for SentryAppComponents: Some SentryAppComponents will reach out to a specified URI to get dropdown options from. If the request to the URI fails, currently we catch the failure, raise an exception and in the SentryAppComponentEndpoint and append the
component_uuidto an errors list. Then in the serializer we check the errors list to see if the serialized component's uuid is present and if so we set an error flag on the component to True. The frontend then reads the returned json and if the error flag is True, we disable the flagged component (grey out).Tested all the errors & messages, but this can go in after broader changes to errors for SentryApps outlined here