diff --git a/.changeset/tiny-panthers-jump.md b/.changeset/tiny-panthers-jump.md new file mode 100644 index 0000000000..d059b826da --- /dev/null +++ b/.changeset/tiny-panthers-jump.md @@ -0,0 +1,22 @@ +--- +'@backstage/backend-common': patch +'@backstage/cli': patch +'@backstage/config-loader': patch +'@backstage/core-app-api': patch +'@backstage/core-components': patch +'@backstage/core-plugin-api': patch +'@backstage/create-app': patch +'@backstage/techdocs-common': patch +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-catalog-backend': patch +'@backstage/plugin-catalog-backend-module-ldap': patch +'@backstage/plugin-catalog-import': patch +'@backstage/plugin-catalog-react': patch +'@backstage/plugin-code-coverage-backend': patch +'@backstage/plugin-kubernetes-backend': patch +'@backstage/plugin-scaffolder-backend': patch +'@backstage/plugin-techdocs-backend': patch +--- + +Internal updates to apply more strict checks to throw errors. diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index f6e42d1945..62de2e001e 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -17,6 +17,7 @@ import knexFactory, { Knex } from 'knex'; import { Config } from '@backstage/config'; +import { ForwardedError } from '@backstage/errors'; import { mergeDatabaseConfig } from '../config'; import { DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; @@ -94,8 +95,7 @@ function requirePgConnectionString() { try { return require('pg-connection-string').parse; } catch (e) { - const message = `Postgres: Install 'pg-connection-string'`; - throw new Error(`${message}\n${e.message}`); + throw new ForwardedError("Postgres: Install 'pg-connection-string'", e); } } diff --git a/packages/backend-common/src/reading/AwsS3UrlReader.test.ts b/packages/backend-common/src/reading/AwsS3UrlReader.test.ts index 0bc0e4f97f..6959edcc8b 100644 --- a/packages/backend-common/src/reading/AwsS3UrlReader.test.ts +++ b/packages/backend-common/src/reading/AwsS3UrlReader.test.ts @@ -167,7 +167,7 @@ describe('AwsS3UrlReader', () => { ), ).rejects.toThrow( Error( - `Could not retrieve file from S3: not a valid AWS S3 URL: https://test-bucket.s3.us-east-2.NOTamazonaws.com/file.yaml`, + `Could not retrieve file from S3; caused by Error: not a valid AWS S3 URL: https://test-bucket.s3.us-east-2.NOTamazonaws.com/file.yaml`, ), ); }); @@ -216,7 +216,7 @@ describe('AwsS3UrlReader', () => { ), ).rejects.toThrow( Error( - `Could not retrieve file from S3: not a valid AWS S3 URL: https://test-bucket.s3.us-east-2.NOTamazonaws.com/file.yaml`, + `Could not retrieve file from S3; caused by Error: not a valid AWS S3 URL: https://test-bucket.s3.us-east-2.NOTamazonaws.com/file.yaml`, ), ); }); diff --git a/packages/backend-common/src/reading/AwsS3UrlReader.ts b/packages/backend-common/src/reading/AwsS3UrlReader.ts index cb03ec4f71..7fd0f6ba19 100644 --- a/packages/backend-common/src/reading/AwsS3UrlReader.ts +++ b/packages/backend-common/src/reading/AwsS3UrlReader.ts @@ -27,6 +27,7 @@ import { } from './types'; import getRawBody from 'raw-body'; import { AwsS3Integration, ScmIntegrations } from '@backstage/integration'; +import { ForwardedError } from '@backstage/errors'; import { ListObjectsV2Output, ObjectList } from 'aws-sdk/clients/s3'; const parseURL = ( @@ -162,7 +163,7 @@ export class AwsS3UrlReader implements UrlReader { etag: etag, }; } catch (e) { - throw new Error(`Could not retrieve file from S3: ${e.message}`); + throw new ForwardedError('Could not retrieve file from S3', e); } } @@ -203,7 +204,7 @@ export class AwsS3UrlReader implements UrlReader { return await this.deps.treeResponseFactory.fromReadableArray(responses); } catch (e) { - throw new Error(`Could not retrieve file tree from S3: ${e.message}`); + throw new ForwardedError('Could not retrieve file tree from S3', e); } } diff --git a/packages/backend-common/src/reading/integration.test.ts b/packages/backend-common/src/reading/integration.test.ts index ae557be583..951df29605 100644 --- a/packages/backend-common/src/reading/integration.test.ts +++ b/packages/backend-common/src/reading/integration.test.ts @@ -15,6 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; +import { isError } from '@backstage/errors'; import { getVoidLogger } from '../logging'; import { UrlReaders } from './UrlReaders'; @@ -71,7 +72,10 @@ function withRetries(count: number, fn: () => Promise) { error = err; } } - if (!error.message.match(/rate limit|Too Many Requests/)) { + if ( + isError(error) && + !error.message.match(/rate limit|Too Many Requests/) + ) { throw error; } else { console.warn('Request was rate limited', error); diff --git a/packages/backend-common/src/util/DockerContainerRunner.ts b/packages/backend-common/src/util/DockerContainerRunner.ts index da3ce66ec2..b81bd995ca 100644 --- a/packages/backend-common/src/util/DockerContainerRunner.ts +++ b/packages/backend-common/src/util/DockerContainerRunner.ts @@ -16,6 +16,7 @@ import Docker from 'dockerode'; import fs from 'fs-extra'; +import { ForwardedError } from '@backstage/errors'; import { PassThrough } from 'stream'; import { ContainerRunner, RunContainerOptions } from './ContainerRunner'; @@ -45,8 +46,9 @@ export class DockerContainerRunner implements ContainerRunner { try { await this.dockerClient.ping(); } catch (e) { - throw new Error( - `This operation requires Docker. Docker does not appear to be available. Docker.ping() failed with: ${e.message}`, + throw new ForwardedError( + 'This operation requires Docker. Docker does not appear to be available. Docker.ping() failed with', + e, ); } diff --git a/packages/cli/package.json b/packages/cli/package.json index ecc3d3b4e1..ebd8f0169c 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -31,6 +31,7 @@ "@backstage/cli-common": "^0.1.4", "@backstage/config": "^0.1.10", "@backstage/config-loader": "^0.6.10", + "@backstage/errors": "^0.1.2", "@hot-loader/react-dom": "^16.13.0", "@lerna/package-graph": "^4.0.0", "@lerna/project": "^4.0.0", diff --git a/packages/cli/src/commands/create-plugin/createPlugin.ts b/packages/cli/src/commands/create-plugin/createPlugin.ts index 6ddd1f7160..a5ea565cae 100644 --- a/packages/cli/src/commands/create-plugin/createPlugin.ts +++ b/packages/cli/src/commands/create-plugin/createPlugin.ts @@ -24,6 +24,7 @@ import camelCase from 'lodash/camelCase'; import upperFirst from 'lodash/upperFirst'; import os from 'os'; import { Command } from 'commander'; +import { assertError } from '@backstage/errors'; import { parseOwnerIds, addCodeownersEntry, @@ -173,6 +174,7 @@ async function buildPlugin(pluginFolder: string) { ); }); } catch (error) { + assertError(error); Task.error(error.message); break; } @@ -329,6 +331,7 @@ export default async (cmd: Command) => { Task.log(); Task.exit(); } catch (error) { + assertError(error); Task.error(error.message); Task.log('It seems that something went wrong when creating the plugin 🤔'); diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index d6686f2655..302f76cc5d 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { assertError } from '@backstage/errors'; import { CommanderStatic } from 'commander'; import { exitWithError } from '../lib/errors'; @@ -252,6 +253,7 @@ function lazy( process.exit(0); } catch (error) { + assertError(error); exitWithError(error); } }; diff --git a/packages/cli/src/commands/remove-plugin/removePlugin.ts b/packages/cli/src/commands/remove-plugin/removePlugin.ts index 9e1e18bb7b..11fdc8bb11 100644 --- a/packages/cli/src/commands/remove-plugin/removePlugin.ts +++ b/packages/cli/src/commands/remove-plugin/removePlugin.ts @@ -20,6 +20,7 @@ import inquirer, { Answers, Question } from 'inquirer'; import { getCodeownersFilePath } from '../../lib/codeowners'; import { paths } from '../../lib/paths'; import { Task } from '../../lib/tasks'; +import { assertError } from '@backstage/errors'; const BACKSTAGE = '@backstage'; @@ -35,6 +36,7 @@ export const checkExists = async (rootDir: string, pluginName: string) => { ); } } catch (e) { + assertError(e); throw new Error( chalk.red( ` There was an error removing plugin ${chalk.cyan(pluginName)}: ${ @@ -51,6 +53,7 @@ export const removePluginDirectory = async (destination: string) => { try { await fse.remove(destination); } catch (e) { + assertError(e); throw Error( chalk.red( ` There was a problem removing the plugin directory: ${e.message}`, @@ -67,6 +70,7 @@ export const removeSymLink = async (destination: string) => { try { await fse.remove(destination); } catch (e) { + assertError(e); throw Error( chalk.red( ` Could not remove symbolic link\t${chalk.cyan(destination)}: ${ @@ -106,6 +110,7 @@ export const removeReferencesFromPluginsFile = async ( try { await removeAllStatementsContainingID(pluginsFile, pluginNameCapitalized); } catch (e) { + assertError(e); throw new Error( chalk.red( ` There was an error removing export statement for plugin ${chalk.cyan( @@ -125,6 +130,7 @@ export const removePluginFromCodeOwners = async ( try { await removeAllStatementsContainingID(codeOwnersFile, pluginName); } catch (e) { + assertError(e); throw new Error( chalk.red( ` There was an error removing code owners statement for plugin ${chalk.cyan( @@ -165,6 +171,7 @@ export const removeReferencesFromAppPackage = async ( 'utf-8', ); } catch (e) { + assertError(e); throw new Error( chalk.red( ` Failed to remove plugin as dependency in app: ${chalk.cyan( @@ -245,6 +252,7 @@ export default async () => { ); Task.log(); } catch (error) { + assertError(error); Task.error(error.message); Task.log('It seems that something went wrong when removing the plugin 🤔'); } diff --git a/packages/cli/src/commands/versions/bump.ts b/packages/cli/src/commands/versions/bump.ts index 257a83e2be..7b967839b2 100644 --- a/packages/cli/src/commands/versions/bump.ts +++ b/packages/cli/src/commands/versions/bump.ts @@ -17,6 +17,7 @@ import fs from 'fs-extra'; import chalk from 'chalk'; import semver from 'semver'; +import { isError } from '@backstage/errors'; import { resolve as resolvePath } from 'path'; import { run } from '../../lib/run'; import { paths } from '../../lib/paths'; @@ -59,7 +60,7 @@ export default async () => { try { target = await findTargetVersion(name); } catch (error) { - if (error.name === 'NotFoundError') { + if (isError(error) && error.name === 'NotFoundError') { console.log(`Package info not found, ignoring package ${name}`); return; } @@ -97,7 +98,7 @@ export default async () => { try { target = await findTargetVersion(name); } catch (error) { - if (error.name === 'NotFoundError') { + if (isError(error) && error.name === 'NotFoundError') { console.log(`Package info not found, ignoring package ${name}`); return; } diff --git a/packages/cli/src/lib/run.ts b/packages/cli/src/lib/run.ts index 7cf22a6df9..4efb791c30 100644 --- a/packages/cli/src/lib/run.ts +++ b/packages/cli/src/lib/run.ts @@ -23,6 +23,7 @@ import { import { ExitCodeError } from './errors'; import { promisify } from 'util'; import { LogFunc } from './logging'; +import { assertError, ForwardedError } from '@backstage/errors'; const execFile = promisify(execFileCb); @@ -75,10 +76,14 @@ export async function runPlain(cmd: string, ...args: string[]) { const { stdout } = await execFile(cmd, args, { shell: true }); return stdout.trim(); } catch (error) { - if (error.stderr) { - process.stderr.write(error.stderr); + assertError(error); + if ('stderr' in error) { + process.stderr.write(error.stderr as Buffer); } - throw new ExitCodeError(error.code, [cmd, ...args].join(' ')); + if (typeof error.code === 'number') { + throw new ExitCodeError(error.code, [cmd, ...args].join(' ')); + } + throw new ForwardedError('Unknown execution error', error); } } diff --git a/packages/config-loader/package.json b/packages/config-loader/package.json index a719c6389b..ebca3ffdf3 100644 --- a/packages/config-loader/package.json +++ b/packages/config-loader/package.json @@ -32,6 +32,7 @@ "dependencies": { "@backstage/cli-common": "^0.1.4", "@backstage/config": "^0.1.9", + "@backstage/errors": "^0.1.2", "@types/json-schema": "^7.0.6", "ajv": "^7.0.3", "chokidar": "^3.5.2", diff --git a/packages/config-loader/src/lib/env.ts b/packages/config-loader/src/lib/env.ts index b244c06c64..57cfc077a7 100644 --- a/packages/config-loader/src/lib/env.ts +++ b/packages/config-loader/src/lib/env.ts @@ -15,6 +15,7 @@ */ import { AppConfig, JsonObject } from '@backstage/config'; +import { assertError } from '@backstage/errors'; const ENV_PREFIX = 'APP_CONFIG_'; @@ -96,6 +97,7 @@ function safeJsonParse(str: string): [Error | null, any] { try { return [null, JSON.parse(str)]; } catch (err) { + assertError(err); return [err, str]; } } diff --git a/packages/config-loader/src/lib/schema/collect.ts b/packages/config-loader/src/lib/schema/collect.ts index f8b134119b..de6aff376f 100644 --- a/packages/config-loader/src/lib/schema/collect.ts +++ b/packages/config-loader/src/lib/schema/collect.ts @@ -24,6 +24,7 @@ import { import { ConfigSchemaPackageEntry } from './types'; import { getProgramFromFiles, generateSchema } from 'typescript-json-schema'; import { JsonObject } from '@backstage/config'; +import { assertError } from '@backstage/errors'; type Item = { name?: string; @@ -189,6 +190,7 @@ function compileTsSchemas(paths: string[]) { [path.split(sep).join('/')], // Unix paths are expected for all OSes here ) as JsonObject | null; } catch (error) { + assertError(error); if (error.message !== 'type Config not found') { throw error; } diff --git a/packages/config-loader/src/lib/transform/apply.ts b/packages/config-loader/src/lib/transform/apply.ts index 690d280266..091f9c23c5 100644 --- a/packages/config-loader/src/lib/transform/apply.ts +++ b/packages/config-loader/src/lib/transform/apply.ts @@ -15,6 +15,7 @@ */ import { JsonObject, JsonValue } from '@backstage/config'; +import { assertError } from '@backstage/errors'; import { TransformFunc } from './types'; import { isObject } from './utils'; @@ -46,6 +47,7 @@ export async function applyConfigTransforms( break; } } catch (error) { + assertError(error); throw new Error(`error at ${path}, ${error.message}`); } } diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 315b44c4ee..4abcc2e314 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -19,6 +19,7 @@ import yaml from 'yaml'; import chokidar from 'chokidar'; import { resolve as resolvePath, dirname, isAbsolute, basename } from 'path'; import { AppConfig } from '@backstage/config'; +import { ForwardedError } from '@backstage/errors'; import { applyConfigTransforms, readEnvConfig, @@ -114,9 +115,7 @@ export async function loadConfig( try { fileConfigs = await loadConfigFiles(); } catch (error) { - throw new Error( - `Failed to read static configuration file, ${error.message}`, - ); + throw new ForwardedError('Failed to read static configuration file', error); } const envConfigs = await readEnvConfig(process.env); diff --git a/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.test.ts b/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.test.ts index 9cd666e8a3..7fcbde97df 100644 --- a/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.test.ts +++ b/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.test.ts @@ -45,25 +45,25 @@ describe('UrlPatternDiscovery', () => { it('should validate that the pattern is a valid URL', () => { expect(() => { UrlPatternDiscovery.compile('example.com'); - }).toThrow('Invalid discovery URL pattern, Invalid URL: example.com'); + }).toThrow("Invalid discovery URL pattern, URL 'example.com' is invalid"); expect(() => { UrlPatternDiscovery.compile('http://'); - }).toThrow('Invalid discovery URL pattern, Invalid URL: http://'); + }).toThrow("Invalid discovery URL pattern, URL 'http://' is invalid"); expect(() => { UrlPatternDiscovery.compile('abc123'); - }).toThrow('Invalid discovery URL pattern, Invalid URL: abc123'); + }).toThrow("Invalid discovery URL pattern, URL 'abc123' is invalid"); expect(() => { UrlPatternDiscovery.compile('http://example.com:{{pluginId}}'); }).toThrow( - 'Invalid discovery URL pattern, Invalid URL: http://example.com:pluginId', + "Invalid discovery URL pattern, URL 'http://example.com:pluginId' is invalid", ); expect(() => { UrlPatternDiscovery.compile('/{{pluginId}}'); - }).toThrow('Invalid discovery URL pattern, Invalid URL: /pluginId'); + }).toThrow("Invalid discovery URL pattern, URL '/pluginId' is invalid"); expect(() => { UrlPatternDiscovery.compile('http://localhost/{{pluginId}}?forbidden'); diff --git a/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.ts b/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.ts index 81cc5a26b1..58e14c9f04 100644 --- a/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.ts +++ b/packages/core-app-api/src/apis/implementations/DiscoveryApi/UrlPatternDiscovery.ts @@ -16,6 +16,8 @@ import { DiscoveryApi } from '@backstage/core-plugin-api'; +const ERROR_PREFIX = 'Invalid discovery URL pattern,'; + /** * UrlPatternDiscovery is a lightweight DiscoveryApi implementation. * It uses a single template string to construct URLs for each plugin. @@ -30,21 +32,22 @@ export class UrlPatternDiscovery implements DiscoveryApi { */ static compile(pattern: string): UrlPatternDiscovery { const parts = pattern.split(/\{\{\s*pluginId\s*\}\}/); + const urlStr = parts.join('pluginId'); + let url; try { - const urlStr = parts.join('pluginId'); - const url = new URL(urlStr); - if (url.hash) { - throw new Error('URL must not have a hash'); - } - if (url.search) { - throw new Error('URL must not have a query'); - } - if (urlStr.endsWith('/')) { - throw new Error('URL must not end with a slash'); - } - } catch (error) { - throw new Error(`Invalid discovery URL pattern, ${error.message}`); + url = new URL(urlStr); + } catch { + throw new Error(`${ERROR_PREFIX} URL '${urlStr}' is invalid`); + } + if (url.hash) { + throw new Error(`${ERROR_PREFIX} URL must not have a hash`); + } + if (url.search) { + throw new Error(`${ERROR_PREFIX} URL must not have a query`); + } + if (urlStr.endsWith('/')) { + throw new Error(`${ERROR_PREFIX} URL must not end with a slash`); } return new UrlPatternDiscovery(parts); diff --git a/packages/core-app-api/src/routing/RoutingProvider.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.test.tsx index 12d9e9ee33..fbdd34ae68 100644 --- a/packages/core-app-api/src/routing/RoutingProvider.test.tsx +++ b/packages/core-app-api/src/routing/RoutingProvider.test.tsx @@ -83,7 +83,7 @@ const MockRouteSource = (props: { } catch (ex) { return (
- Error at {props.name}: {ex.message} + Error at {props.name}, {String(ex)}
); } @@ -293,12 +293,12 @@ describe('discovery', () => { expect( rendered.getByText( - `Error at outsideWithParams: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, + `Error at outsideWithParams, Error: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, ), ).toBeInTheDocument(); expect( rendered.getByText( - `Error at outsideNoParams: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, + `Error at outsideNoParams, Error: Cannot route to ${ref3} with parent ${ref5} as it has parameters`, ), ).toBeInTheDocument(); }); diff --git a/packages/core-components/src/components/OAuthRequestDialog/LoginRequestListItem.tsx b/packages/core-components/src/components/OAuthRequestDialog/LoginRequestListItem.tsx index b911a9a1d9..ef0d2373e4 100644 --- a/packages/core-components/src/components/OAuthRequestDialog/LoginRequestListItem.tsx +++ b/packages/core-components/src/components/OAuthRequestDialog/LoginRequestListItem.tsx @@ -21,6 +21,7 @@ import ListItemText from '@material-ui/core/ListItemText'; import Typography from '@material-ui/core/Typography'; import Button from '@material-ui/core/Button'; import React, { useState } from 'react'; +import { isError } from '@backstage/errors'; import { PendingAuthRequest } from '@backstage/core-plugin-api'; export type LoginRequestListItemClassKey = 'root'; @@ -42,14 +43,14 @@ type RowProps = { const LoginRequestListItem = ({ request, busy, setBusy }: RowProps) => { const classes = useItemStyles(); - const [error, setError] = useState(); + const [error, setError] = useState(); const handleContinue = async () => { setBusy(true); try { await request.trigger(); } catch (e) { - setError(e); + setError(isError(e) ? e.message : 'An unspecified error occurred'); } finally { setBusy(false); } @@ -64,13 +65,7 @@ const LoginRequestListItem = ({ request, busy, setBusy }: RowProps) => { - {error.message || 'An unspecified error occurred'} - - ) - } + secondary={error && {error}} />