Merge pull request backstage#326 from spotify/rugvip/errors

Add initial version of core error API with small showcase in welcome plugin and app

packages/app/package.json

@@ -5,6 +5,7 @@
   "dependencies": {
     "@material-ui/core": "^4.9.1",
     "@material-ui/icons": "^4.9.1",
+    "@material-ui/lab": "4.0.0-alpha.45",
     "@spotify-backstage/cli": "^0.1.0",
     "@spotify-backstage/core": "^0.1.0",
     "@spotify-backstage/plugin-home-page": "^0.1.0",

packages/app/src/App.tsx

@@ -19,7 +19,9 @@ import { BackstageTheme, createApp } from '@spotify-backstage/core';
 import React, { FC } from 'react';
 import { BrowserRouter as Router } from 'react-router-dom';
 import Root from './components/Root';
+import ErrorDisplay from './components/ErrorDisplay';
 import * as plugins from './plugins';
+import apis, { errorDialogForwarder } from './apis';
 
 const useStyles = makeStyles(theme => ({
   '@global': {
@@ -40,6 +42,7 @@ const useStyles = makeStyles(theme => ({
 }));
 
 const app = createApp();
+app.registerApis(apis);
 app.registerPlugin(...Object.values(plugins));
 const AppComponent = app.build();
 
@@ -48,6 +51,7 @@ const App: FC<{}> = () => {
   return (
     <CssBaseline>
       <ThemeProvider theme={BackstageTheme}>
+        <ErrorDisplay forwarder={errorDialogForwarder} />
         <Router>
           <Root>
             <AppComponent />

packages/app/src/apis.ts

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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 { ApiHolder, ApiRegistry, errorApiRef } from '@spotify-backstage/core';
+import { ErrorDisplayForwarder } from './components/ErrorDisplay/ErrorDisplay';
+
+const builder = ApiRegistry.builder();
+
+export const errorDialogForwarder = new ErrorDisplayForwarder();
+
+builder.add(errorApiRef, errorDialogForwarder);
+
+export default builder.build() as ApiHolder;

packages/app/src/components/ErrorDisplay/ErrorDisplay.tsx

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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, { FC, useEffect, useState } from 'react';
+import PropTypes from 'prop-types';
+import { Snackbar } from '@material-ui/core';
+import { Alert } from '@material-ui/lab';
+import { ErrorApi, ErrorContext } from '@spotify-backstage/core';
+
+type SubscriberFunc = (error: Error) => void;
+type Unsubscribe = () => void;
+
+// TODO: figure out where to put implementations of APIs, both inside apps
+// but also in core/separate package.
+export class ErrorDisplayForwarder implements ErrorApi {
+  private readonly subscribers = new Set<SubscriberFunc>();
+
+  post(error: Error, context?: ErrorContext) {
+    if (context?.hidden) {
+      return;
+    }
+
+    this.subscribers.forEach(subscriber => subscriber(error));
+  }
+
+  subscribe(func: SubscriberFunc): Unsubscribe {
+    this.subscribers.add(func);
+
+    return () => {
+      this.subscribers.delete(func);
+    };
+  }
+}
+
+type Props = {
+  forwarder: ErrorDisplayForwarder;
+};
+
+// TODO: improve on this and promote to a shared component for use by all apps.
+const ErrorDisplay: FC<Props> = ({ forwarder }) => {
+  const [errors, setErrors] = useState<Array<Error>>([]);
+
+  useEffect(() => {
+    return forwarder.subscribe(error => setErrors(errs => errs.concat(error)));
+  }, [forwarder]);
+
+  if (errors.length === 0) {
+    return null;
+  }
+
+  const [firstError] = errors;
+
+  const handleClose = () => {
+    setErrors(errs => errs.filter(err => err !== firstError));
+  };
+
+  return (
+    <Snackbar
+      open
+      message={firstError.toString()}
+      anchorOrigin={{ vertical: 'top', horizontal: 'center' }}
+    >
+      <Alert onClose={handleClose} severity="error">
+        {firstError.toString()}
+      </Alert>
+    </Snackbar>
+  );
+};
+
+ErrorDisplay.propTypes = {
+  forwarder: PropTypes.instanceOf(ErrorDisplayForwarder).isRequired,
+};
+
+export default ErrorDisplay;

packages/app/src/components/ErrorDisplay/index.ts

@@ -0,0 +1,17 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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.
+ */
+
+export { default } from './ErrorDisplay';

packages/core/src/api/apis/ApiRef.test.ts

@@ -25,14 +25,26 @@ describe('ApiRef', () => {
     expect(() => ref.T).toThrow('tried to read ApiRef.T of apiRef{abc}');
   });
 
-  it('should require a ascii letters only in id', () => {
-    for (const id of ['a', 'abc', 'ABC', 'aBC', 'aBc']) {
+  it('should reject invalid ids', () => {
+    for (const id of ['a', 'abc', 'a.b.c', 'ab.c', 'abc.abc.abc3']) {
       expect(new ApiRef({ id, description: '123' }).id).toBe(id);
     }
 
-    for (const id of ['123', 'ab-c', 'ab_c', 'a2c', '', '_']) {
+    for (const id of [
+      '123',
+      'ab-c',
+      'ab_c',
+      '.',
+      '2ac',
+      'ab.3a',
+      '.abc',
+      'abc.',
+      'ab..s',
+      '',
+      '_',
+    ]) {
       expect(() => new ApiRef({ id, description: '123' }).id).toThrow(
-        `API id must only contain ascii letters, got '${id}'`,
+        `API id must only contain lowercase alphanums separated by dots, got '${id}'`,
       );
     }
   });

packages/core/src/api/apis/ApiRef.ts

@@ -21,9 +21,9 @@ export type ApiRefConfig = {
 
 export default class ApiRef<T> {
   constructor(private readonly config: ApiRefConfig) {
-    if (!config.id.match(/^[a-zA-Z]+$/)) {
+    if (!config.id.match(/^[a-z][a-z0-9]*(\.[a-z][a-z0-9]*)*$/)) {
       throw new Error(
-        `API id must only contain ascii letters, got '${config.id}'`,
+        `API id must only contain lowercase alphanums separated by dots, got '${config.id}'`,
       );
     }
   }

packages/core/src/api/apis/ApiRegistry.ts

@@ -22,7 +22,7 @@ type ApiImpl<T = unknown> = readonly [ApiRef<T>, T];
 class ApiRegistryBuilder {
   private apis: ApiImpl[] = [];
 
-  add<T>(api: ApiRef<T>, impl: T): T {
+  add<T, I extends T>(api: ApiRef<T>, impl: I): I {
     this.apis.push([api, impl]);
     return impl;
   }

packages/core/src/api/apis/definitions/error.ts

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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 ApiRef from '../ApiRef';
+
+/**
+ * Mirrors the javascript Error class, for the purpose of
+ * providing documentation and optional fields.
+ */
+type Error = {
+  name: string;
+  message: string;
+  stack?: string;
+};
+
+/**
+ * Provides additional information about an error that was posted to the application.
+ */
+export type ErrorContext = {
+  // If set to true, this error should not be displayed to the user. Defaults to false.
+  hidden?: boolean;
+};
+
+/**
+ * The error API is used to report errors to the app, and display them to the user.
+ *
+ * Plugins can use this API as a method of displaying errors to the user, but also
+ * to report errors for collection by error reporting services.
+ *
+ * If an error can be displayed inline, e.g. as feedback in a form, that should be
+ * preferred over relying on this API to display the error. The main use of this api
+ * for displaying errors should be for asynchronous errors, such as a failing background process.
+ *
+ * Even if an error is displayed inline, it should still be reported through this api
+ * if it would be useful to collect or log it for debugging purposes, but with
+ * the hidden flag set. For example, an error arising from form field validation
+ * should probably not be reported, while a failed REST call would be useful to report.
+ */
+export type ErrorApi = {
+  /**
+   * Post an error for handling by the application.
+   */
+  post(error: Error, context?: ErrorContext);
+};
+
+export const errorApiRef = new ApiRef<ErrorApi>({
+  id: 'core.error',
+  description: 'Used to report errors and forward them to the app',
+});

packages/core/src/api/apis/definitions/index.ts

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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.
+ */
+
+// This folder contains definitions for all core APIs.
+//
+// Plugins should rely on these APIs for functionality as much as possible.
+//
+// If you think some API definition is missing, please open an Issue or send a PR!
+
+export * from './error';

packages/core/src/api/apis/index.ts

@@ -18,3 +18,4 @@ export { default as ApiProvider, useApi } from './ApiProvider';
 export { default as ApiRegistry } from './ApiRegistry';
 export { default as ApiTestRegistry } from './ApiTestRegistry';
 export * from './types';
+export * from './definitions';

packages/core/src/api/app/AppBuilder.tsx

@@ -25,6 +25,7 @@ import {
   SystemIconKey,
   defaultSystemIcons,
 } from '../../icons';
+import { ApiHolder, ApiProvider } from '../apis';
 import LoginPage from './LoginPage';
 
 class AppImpl implements App {
@@ -36,9 +37,14 @@ class AppImpl implements App {
 }
 
 export default class AppBuilder {
+  private apis?: ApiHolder;
   private systemIcons = { ...defaultSystemIcons };
   private readonly plugins = new Set<BackstagePlugin>();
 
+  registerApis(apis: ApiHolder) {
+    this.apis = apis;
+  }
+
   registerIcons(icons: Partial<SystemIcons>) {
     this.systemIcons = { ...this.systemIcons, ...icons };
   }
@@ -91,13 +97,17 @@ export default class AppBuilder {
       <Route key="login" path="/login" component={LoginPage} exact />,
     );
 
-    return () => (
-      <AppContextProvider app={app}>
-        <Switch>
-          {routes}
-          <Route component={() => <span>404 Not Found</span>} />
-        </Switch>
-      </AppContextProvider>
+    let rendered = (
+      <Switch>
+        {routes}
+        <Route component={() => <span>404 Not Found</span>} />
+      </Switch>
     );
+
+    if (this.apis) {
+      rendered = <ApiProvider apis={this.apis} children={rendered} />;
+    }
+
+    return () => <AppContextProvider app={app} children={rendered} />;
   }
 }