etr2460 commented on issue #9194: [SIP-40] Proposal for Custom Error Messages URL: https://github.com/apache/incubator-superset/issues/9194#issuecomment-590549191 @kristw: One concern I had about moving this out into a Superset error package was that I don't think strings for default error messages would get translated in those packages. That seemed like a non-starter to me, so I didn't consider it. If you have a way to resolve this, then I'd be happy to hear it! For examples of fragmented error handling, in SQL Lab we render alerts based on error messages injected inside the query object (as well as the poorly defined `link` parameter): https://github.com/apache/incubator-superset/blob/291306392443a5a0d0e2ee0cc4a95d37c56d4589/superset-frontend/src/SqlLab/components/ResultSet.jsx#L201-L212 but in Charts we render them based off the `chartAlert` param (in a completely different component too): https://github.com/apache/incubator-superset/blob/291306392443a5a0d0e2ee0cc4a95d37c56d4589/superset-frontend/src/chart/Chart.jsx#L136-L143 @craig-rueda: I totally agree with an `errors` array inside the structure, that makes a bunch of sense. We'll need to consider what the default UI experience is (multiple alerts? merging errors?) but I think it'll be worth it. One thing I don't agree with is using an error code as opposed to an enum. This is for two reasons: - The error code in the response would be easily confused with a http status code that's already an int. - By using an Enum, we can both provide some idea of what the error is to the user, but also provide a unique string for searching error resolutions online. Instead of getting a `Superset Error 1432` you'd get a `Superset Error PRESTO_INTERNAL_SERVER_ERROR` which i think is much more readable and could save the extra trouble of mapping a code to resolutions online. The enum provides additional value to the error message as an error like this might include the message: `Presto encountered an unknown issue when running this query. Please try again later.` and the enum specifies a more technical error
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
