This is an automated email from the ASF dual-hosted git repository. erikrit pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push: new 5d7b135 feat(errors): add client scaffolding for custom error messages (#9677) 5d7b135 is described below commit 5d7b13507e207ac06793034208b2d28b4140373b Author: Erik Ritter <erik.rit...@airbnb.com> AuthorDate: Wed Apr 29 17:20:31 2020 -0700 feat(errors): add client scaffolding for custom error messages (#9677) --- .../getErrorMessageComponentRegistry_spec.tsx | 64 ++++++++++++++++++++++ superset-frontend/src/chart/Chart.jsx | 2 +- superset-frontend/src/components/ErrorBoundary.jsx | 2 +- .../ErrorMessageWithStackTrace.tsx | 22 +++++++- .../getErrorMessageComponentRegistry.ts | 38 +++++++++++++ .../src/components/ErrorMessage/types.ts | 47 ++++++++++++++++ superset-frontend/src/setup/setupApp.ts | 4 ++ superset-frontend/src/setup/setupErrorMessages.ts | 24 ++++++++ .../src/setup/setupErrorMessagesExtra.ts | 21 +++++++ 9 files changed, 219 insertions(+), 5 deletions(-) diff --git a/superset-frontend/spec/javascripts/components/ErrorMessage/getErrorMessageComponentRegistry_spec.tsx b/superset-frontend/spec/javascripts/components/ErrorMessage/getErrorMessageComponentRegistry_spec.tsx new file mode 100644 index 0000000..b4a88f6 --- /dev/null +++ b/superset-frontend/spec/javascripts/components/ErrorMessage/getErrorMessageComponentRegistry_spec.tsx @@ -0,0 +1,64 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import getErrorMessageComponentRegistry from 'src/components/ErrorMessage/getErrorMessageComponentRegistry'; +import { ErrorMessageComponentProps } from 'src/components/ErrorMessage/types'; + +const ERROR_MESSAGE_COMPONENT = (_: ErrorMessageComponentProps) => ( + <div>Test error</div> +); + +const OVERRIDE_ERROR_MESSAGE_COMPONENT = (_: ErrorMessageComponentProps) => ( + <div>Custom error</div> +); + +describe('getErrorMessageComponentRegistry', () => { + it('returns undefined for a non existent key', () => { + expect(getErrorMessageComponentRegistry().get('INVALID_KEY')).toEqual( + undefined, + ); + }); + + it('returns a component for a set key', () => { + getErrorMessageComponentRegistry().registerValue( + 'VALID_KEY', + ERROR_MESSAGE_COMPONENT, + ); + + expect(getErrorMessageComponentRegistry().get('VALID_KEY')).toEqual( + ERROR_MESSAGE_COMPONENT, + ); + }); + + it('returns the correct component for an overridden key', () => { + getErrorMessageComponentRegistry().registerValue( + 'OVERRIDE_KEY', + ERROR_MESSAGE_COMPONENT, + ); + + getErrorMessageComponentRegistry().registerValue( + 'OVERRIDE_KEY', + OVERRIDE_ERROR_MESSAGE_COMPONENT, + ); + + expect(getErrorMessageComponentRegistry().get('OVERRIDE_KEY')).toEqual( + OVERRIDE_ERROR_MESSAGE_COMPONENT, + ); + }); +}); diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 7c19e78..51d392b 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -24,7 +24,7 @@ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import { Logger, LOG_ACTIONS_RENDER_CHART_CONTAINER } from '../logger/LogUtils'; import Loading from '../components/Loading'; import RefreshChartOverlay from '../components/RefreshChartOverlay'; -import ErrorMessageWithStackTrace from '../components/ErrorMessageWithStackTrace'; +import ErrorMessageWithStackTrace from '../components/ErrorMessage/ErrorMessageWithStackTrace'; import ErrorBoundary from '../components/ErrorBoundary'; import ChartRenderer from './ChartRenderer'; import './chart.less'; diff --git a/superset-frontend/src/components/ErrorBoundary.jsx b/superset-frontend/src/components/ErrorBoundary.jsx index bb1d916..f1fa942 100644 --- a/superset-frontend/src/components/ErrorBoundary.jsx +++ b/superset-frontend/src/components/ErrorBoundary.jsx @@ -19,7 +19,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { t } from '@superset-ui/translation'; -import ErrorMessageWithStackTrace from './ErrorMessageWithStackTrace'; +import ErrorMessageWithStackTrace from './ErrorMessage/ErrorMessageWithStackTrace'; const propTypes = { children: PropTypes.node.isRequired, diff --git a/superset-frontend/src/components/ErrorMessageWithStackTrace.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx similarity index 75% rename from superset-frontend/src/components/ErrorMessageWithStackTrace.tsx rename to superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx index 49403bd..6ffe586 100644 --- a/superset-frontend/src/components/ErrorMessageWithStackTrace.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx @@ -19,19 +19,35 @@ import React, { useState } from 'react'; // @ts-ignore import { Alert, Collapse } from 'react-bootstrap'; +import getErrorMessageComponentRegistry from './getErrorMessageComponentRegistry'; +import { SupersetError } from './types'; -interface Props { - message: string; +type Props = { + error?: SupersetError; link?: string; + message: string; stackTrace?: string; -} +}; export default function ErrorMessageWithStackTrace({ + error, message, link, stackTrace, }: Props) { const [showStackTrace, setShowStackTrace] = useState(false); + + // Check if a custom error message component was registered for this message + if (error) { + const ErrorMessageComponent = getErrorMessageComponentRegistry().get( + error.errorType, + ); + if (ErrorMessageComponent) { + return <ErrorMessageComponent error={error} />; + } + } + + // Fallback to the default error message renderer return ( <div className={`stack-trace-container${stackTrace ? ' has-trace' : ''}`}> <Alert diff --git a/superset-frontend/src/components/ErrorMessage/getErrorMessageComponentRegistry.ts b/superset-frontend/src/components/ErrorMessage/getErrorMessageComponentRegistry.ts new file mode 100644 index 0000000..e21a9d3 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/getErrorMessageComponentRegistry.ts @@ -0,0 +1,38 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Registry, makeSingleton, OverwritePolicy } from '@superset-ui/core'; +import { ErrorMessageComponent } from './types'; + +class ErrorMessageComponentRegistry extends Registry< + ErrorMessageComponent, + ErrorMessageComponent +> { + constructor() { + super({ + name: 'ErrorMessageComponent', + overwritePolicy: OverwritePolicy.ALLOW, + }); + } +} + +const getErrorMessageComponentRegistry = makeSingleton( + ErrorMessageComponentRegistry, +); + +export default getErrorMessageComponentRegistry; diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts new file mode 100644 index 0000000..e2cea5b --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -0,0 +1,47 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// TODO: Add more error types as we classify more errors +export const ErrorTypeEnum = { + // Generic errors created on the frontend + 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 ErrorTypeEnum>; + +export type ErrorLevel = 'info' | 'warning' | 'error'; + +export type SupersetError = { + errorType: ErrorType; + extra: Record<string, any>; + level: ErrorLevel; + message: string; +}; + +export type ErrorMessageComponentProps = { + error: SupersetError; +}; + +export type ErrorMessageComponent = React.ComponentType< + ErrorMessageComponentProps +>; diff --git a/superset-frontend/src/setup/setupApp.ts b/superset-frontend/src/setup/setupApp.ts index 962799f..e81c6d2 100644 --- a/superset-frontend/src/setup/setupApp.ts +++ b/superset-frontend/src/setup/setupApp.ts @@ -20,6 +20,7 @@ import $ from 'jquery'; import { SupersetClient } from '@superset-ui/connection'; import getClientErrorObject from '../utils/getClientErrorObject'; +import setupErrorMessages from './setupErrorMessages'; function showApiMessage(resp: { severity?: string; message: string }) { const template = @@ -84,4 +85,7 @@ export default function setupApp() { // @ts-ignore window.jQuery = $; require('bootstrap'); + + // setup appwide custom error messages + setupErrorMessages(); } diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts new file mode 100644 index 0000000..289b3cb --- /dev/null +++ b/superset-frontend/src/setup/setupErrorMessages.ts @@ -0,0 +1,24 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import setupErrorMessagesExtra from './setupErrorMessagesExtra'; + +export default function setupErrorMessages() { + // TODO: Register error messages to the ErrorMessageComponentRegistry once implemented + setupErrorMessagesExtra(); +} diff --git a/superset-frontend/src/setup/setupErrorMessagesExtra.ts b/superset-frontend/src/setup/setupErrorMessagesExtra.ts new file mode 100644 index 0000000..fe90e54 --- /dev/null +++ b/superset-frontend/src/setup/setupErrorMessagesExtra.ts @@ -0,0 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// For individual deployments to add custom error messages +export default function setupErrorMessagesExtra() {}