diff --git a/.changeset/cyan-frogs-count.md b/.changeset/cyan-frogs-count.md new file mode 100644 index 0000000000..b06a07468a --- /dev/null +++ b/.changeset/cyan-frogs-count.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-catalog-backend-module-incremental-ingestion': patch +'@backstage/plugin-devtools-backend': patch +--- + +Remove the error handler middleware, since that is now provided by the framework diff --git a/.changeset/shiny-walls-press.md b/.changeset/shiny-walls-press.md new file mode 100644 index 0000000000..aa93779d85 --- /dev/null +++ b/.changeset/shiny-walls-press.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-defaults': minor +--- + +**BREAKING**: Ensure that an error handler middleware exists at the end of each plugin `httpRouter` handler chain. This makes it so that exceptions thrown by plugin routes are caught and encoded in the standard error format. + +If you were using the standard `MiddlewareFactory` just to put an `error` middleware in you router, you can now remove that at your earliest convenience since it's redundant. If you have custom error handlers in your plugin router, those will continue to function as previously. If you were relying on thrown errors propagating all the way down to the root HTTP router, you will find that they no longer do that, and may want to hoist your error handling up to the plugin level instead. diff --git a/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts b/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts index 6f8792aa3d..1f2b989018 100644 --- a/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts @@ -27,6 +27,7 @@ import { createCredentialsBarrier, createAuthIntegrationRouter, } from './http'; +import { MiddlewareFactory } from '../rootHttpRouter'; /** * HTTP route registration for plugins. @@ -47,8 +48,17 @@ export const httpRouterServiceFactory = createServiceFactory({ rootHttpRouter: coreServices.rootHttpRouter, auth: coreServices.auth, httpAuth: coreServices.httpAuth, + logger: coreServices.logger, }, - async factory({ auth, httpAuth, config, plugin, rootHttpRouter, lifecycle }) { + async factory({ + auth, + httpAuth, + config, + plugin, + rootHttpRouter, + lifecycle, + logger, + }) { const router = PromiseRouter(); rootHttpRouter.use(`/api/${plugin.getId()}`, router); @@ -63,9 +73,15 @@ export const httpRouterServiceFactory = createServiceFactory({ router.use(credentialsBarrier.middleware); router.use(createCookieAuthRefreshMiddleware({ auth, httpAuth })); + const pluginRoutes = PromiseRouter(); + router.use(pluginRoutes); + + const middleware = MiddlewareFactory.create({ config, logger }); + router.use(middleware.error()); + return { use(handler: Handler): void { - router.use(handler); + pluginRoutes.use(handler); }, addAuthPolicy(policy: HttpRouterServiceAuthPolicy): void { credentialsBarrier.addAuthPolicy(policy); diff --git a/plugins/catalog-backend-module-incremental-ingestion/src/module/WrapperProviders.ts b/plugins/catalog-backend-module-incremental-ingestion/src/module/WrapperProviders.ts index 2f8a8df3ba..11b9619dc5 100644 --- a/plugins/catalog-backend-module-incremental-ingestion/src/module/WrapperProviders.ts +++ b/plugins/catalog-backend-module-incremental-ingestion/src/module/WrapperProviders.ts @@ -79,7 +79,6 @@ export class WrapperProviders { return new IncrementalProviderRouter( new IncrementalIngestionDatabaseManager({ client: this.options.client }), this.options.logger, - this.options.config, ).createRouter(); } diff --git a/plugins/catalog-backend-module-incremental-ingestion/src/router/routes.ts b/plugins/catalog-backend-module-incremental-ingestion/src/router/routes.ts index ad8d6ce103..ca6f600840 100644 --- a/plugins/catalog-backend-module-incremental-ingestion/src/router/routes.ts +++ b/plugins/catalog-backend-module-incremental-ingestion/src/router/routes.ts @@ -18,22 +18,17 @@ import express from 'express'; import Router from 'express-promise-router'; import { IncrementalIngestionDatabaseManager } from '../database/IncrementalIngestionDatabaseManager'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; -import { Config } from '@backstage/config'; export class IncrementalProviderRouter { private manager: IncrementalIngestionDatabaseManager; private logger: LoggerService; - private config: Config; constructor( manager: IncrementalIngestionDatabaseManager, logger: LoggerService, - config: Config, ) { this.manager = manager; this.logger = logger; - this.config = config; } createRouter(): express.Router { @@ -253,12 +248,6 @@ export class IncrementalProviderRouter { }, ); - const middleware = MiddlewareFactory.create({ - logger: this.logger, - config: this.config, - }); - router.use(middleware.error()); - return router; } } diff --git a/plugins/catalog-backend-module-incremental-ingestion/src/service/IncrementalCatalogBuilder.ts b/plugins/catalog-backend-module-incremental-ingestion/src/service/IncrementalCatalogBuilder.ts index ed4414b68b..6ea2f10a64 100644 --- a/plugins/catalog-backend-module-incremental-ingestion/src/service/IncrementalCatalogBuilder.ts +++ b/plugins/catalog-backend-module-incremental-ingestion/src/service/IncrementalCatalogBuilder.ts @@ -66,7 +66,6 @@ export class IncrementalCatalogBuilder { const incrementalAdminRouter = await new IncrementalProviderRouter( this.manager, routerLogger, - this.env.config, ).createRouter(); return { incrementalAdminRouter }; diff --git a/plugins/devtools-backend/src/service/router.ts b/plugins/devtools-backend/src/service/router.ts index adcb79c4cd..ad77495b36 100644 --- a/plugins/devtools-backend/src/service/router.ts +++ b/plugins/devtools-backend/src/service/router.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { devToolsConfigReadPermission, @@ -20,7 +21,6 @@ import { devToolsInfoReadPermission, devToolsPermissions, } from '@backstage/plugin-devtools-common'; - import { DevToolsBackendApi } from '../api'; import { NotAllowedError } from '@backstage/errors'; import Router from 'express-promise-router'; @@ -33,7 +33,6 @@ import { PermissionsService, RootConfigService, } from '@backstage/backend-plugin-api'; -import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; /** * @internal @@ -121,8 +120,5 @@ export async function createRouter( response.status(200).json(health); }); - const middleware = MiddlewareFactory.create({ logger, config }); - - router.use(middleware.error()); return router; } diff --git a/plugins/kubernetes-backend/src/routes/resourceRoutes.test.ts b/plugins/kubernetes-backend/src/routes/resourceRoutes.test.ts index 01da602232..7ace17e805 100644 --- a/plugins/kubernetes-backend/src/routes/resourceRoutes.test.ts +++ b/plugins/kubernetes-backend/src/routes/resourceRoutes.test.ts @@ -169,7 +169,7 @@ describe('resourcesRoutes', () => { error: { name: 'InputError', message: 'entity is a required field' }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 400 }, }); @@ -193,7 +193,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 400 }, }); @@ -216,7 +216,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 400 }, }); @@ -240,7 +240,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 401 }, }); @@ -264,7 +264,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 401 }, }); @@ -287,7 +287,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/workloads/query', + url: '/resources/workloads/query', }, response: { statusCode: 500 }, }); @@ -346,7 +346,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -370,7 +370,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -394,7 +394,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -420,7 +420,7 @@ describe('resourcesRoutes', () => { error: { name: 'InputError', message: 'entity is a required field' }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -451,7 +451,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -481,7 +481,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 400 }, }); @@ -512,7 +512,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 401 }, }); @@ -543,7 +543,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 401 }, }); @@ -573,7 +573,7 @@ describe('resourcesRoutes', () => { }, request: { method: 'POST', - url: '/api/kubernetes/resources/custom/query', + url: '/resources/custom/query', }, response: { statusCode: 500 }, }); diff --git a/plugins/notifications-backend/dev/index.ts b/plugins/notifications-backend/dev/index.ts index f1fe453457..936c31f211 100644 --- a/plugins/notifications-backend/dev/index.ts +++ b/plugins/notifications-backend/dev/index.ts @@ -26,7 +26,6 @@ import { } from '@backstage/plugin-notifications-common'; import express, { Response } from 'express'; import Router from 'express-promise-router'; -import { MiddlewareFactory } from '@backstage/backend-defaults/rootHttpRouter'; const randomSeverity = (): NotificationSeverity => { return notificationSeverities[ @@ -77,12 +76,8 @@ const notificationsDebug = createBackendPlugin({ deps: { notifications: notificationService, httpRouter: coreServices.httpRouter, - config: coreServices.rootConfig, - logger: coreServices.logger, }, - async init({ notifications, httpRouter, config, logger }) { - const middleware = MiddlewareFactory.create({ config, logger }); - + async init({ notifications, httpRouter }) { const router = Router(); router.use(express.json()); router.post('/', async (_, res: Response) => { @@ -100,7 +95,6 @@ const notificationsDebug = createBackendPlugin({ }); res.status(200).send({ status: 'ok' }); }); - router.use(middleware.error()); httpRouter.use(router); httpRouter.addAuthPolicy({