feat(errors): add client scaffolding for custom error messages #9677
Conversation
| message, | ||
| link, | ||
| stackTrace, | ||
| }: Props) { | ||
| const [showStackTrace, setShowStackTrace] = useState(false); | ||
|
|
||
| // Check if a custom error message component was registered for this message |
There was a problem hiding this comment.
this is essentially a no-op today as we haven't registered any error messages and this component will never get a new error object passed in.
etr2460
force-pushed
the
erik-ritter--error-messages-1
branch
2 times, most recently
from
cfdf7bb to
c532ec4
Compare
| message, | ||
| link, | ||
| stackTrace, | ||
| }: Props) { | ||
| const [showStackTrace, setShowStackTrace] = useState(false); | ||
|
|
||
| // Check if a custom error message component was registered for this message | ||
| const ErrorMessageComponent = |
There was a problem hiding this comment.
if (error) {
const ErrorMessageComponent = getErrorMessageComponentRegistry().get(error.errorType);
if (ErrorMessageComponent) {
return <ErrorMessageComponent error={error} />
}
}There was a problem hiding this comment.
can modify a bit to check for error once
There was a problem hiding this comment.
works for me! i optimized for repetition over nesting, but i agree this is better
| @@ -84,4 +85,7 @@ export default function setupApp() { | |||
| // @ts-ignore | |||
| window.jQuery = $; | |||
| require('bootstrap'); | |||
|
|
|||
| // setup appwide custom error messages | |||
| setupErrorMessages(); | |||
There was a problem hiding this comment.
not sure if you want to nest the call here or in App.jsx like setupApp and setupPlugins. The items in setupApp here seems likely to be removed at some point.
There was a problem hiding this comment.
I put it here because it seems like there are a bunch of App.jsxs that exist today, and we'd want all of them to have the error message registry attached to them. I assume in the future we'll end up with only one and this file will be removed, but i think it's fine to keep this here for now
| * under the License. | ||
| */ | ||
|
|
||
| // Generic errors created on the frontend |
There was a problem hiding this comment.
Can do this so ErrorType will also work like enum.
// Generic errors created on the frontend
// TODO: Add more error types as we classify more errors
export const ErrorType = {
FRONTEND_CSRF_ERROR: 'FRONTEND_CSRF_ERROR',
FRONTEND_NETWORK_ERROR: 'FRONTEND_NETWORK_ERROR',
FRONTEND_TIMEOUT_ERROR: 'FRONTEND_TIMEOUT_ERROR',
} as const;
type ValueOf<T> = T[keyof T];
export type ErrorType = ValueOf<typeof ErrorType>;| }; | ||
|
|
||
| export type ErrorMessageComponent = | ||
| | React.ComponentType<ErrorMessageComponentProps> |
There was a problem hiding this comment.
React.ComponentType already includes React.FunctionComponent
There was a problem hiding this comment.
Perhaps this should be enough
export type ErrorMessageComponent = ReactComponentType<ErrorMessageComponentProps>;
etr2460
force-pushed
the
erik-ritter--error-messages-1
branch
from
c532ec4 to
dce295f
Compare
mistercrunch
added
🏷️ bot
CATEGORY
Choose one
SUMMARY
Beginning implementation of #9194
This PR adds scaffolding for the client side part of custom error messages. No existing behavior should be changed yet. Scaffolding consists of:
ErrorMessageComponentRegistryfor registering all error messages by error typesetupErrorMessagesExtraNote that this registry is included in the main Superset repo. This is because we only support translations in this repo currently, and it's important to allow all the error messages registered here to have translations.
Todo in future PRs:
TEST PLAN
View a dashboard, ensure network errors and hardcoded raised errors still render properly
ADDITIONAL INFORMATION
REVIEWERS
to: @graceguo-supercat @ktmud @kristw @rusackas