paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix error forwarding in actions registry to preserve original status codes (#33021)

Browse Source 
Signed-off-by: benjdlambert <ben@blam.sh>
This commit is contained in:
Ben Lambert
2026-02-26 09:59:58 +01:00
committed by GitHub
parent c869f0ed9e
commit 62f0a53d65
9 changed files with 345 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/fix-mcp-error-forwarding.md
+7
View File
@@ -0,0 +1,7 @@
---
'@backstage/backend-defaults': patch
'@backstage/backend-test-utils': patch
'@backstage/plugin-mcp-actions-backend': patch
---
Fixed error forwarding in the actions registry so that known errors like `InputError` and `NotFoundError` thrown by actions preserve their original status codes and messages instead of being wrapped in `ForwardedError` and coerced to 500.
packages/backend-defaults/src/alpha/entrypoints/actionsRegistry/DefaultActionsRegistryService.ts
+15 -27
View File
@@ -28,12 +28,7 @@ import {
ActionsRegistryActionOptions,
ActionsRegistryService,
} from '@backstage/backend-plugin-api/alpha';
import {
ForwardedError,
InputError,
NotAllowedError,
NotFoundError,
} from '@backstage/errors';
import { InputError, NotAllowedError, NotFoundError } from '@backstage/errors';
export class DefaultActionsRegistryService implements ActionsRegistryService {
private actions: Map<string, ActionsRegistryActionOptions<any, any>> =
@@ -131,31 +126,24 @@ export class DefaultActionsRegistryService implements ActionsRegistryService {
);
}
try {
const result = await action.action({
input: input.data,
credentials,
logger: this.logger,
});
const result = await action.action({
input: input.data,
credentials,
logger: this.logger,
});
const output = action.schema?.output
? action.schema.output(z).safeParse(result?.output)
: ({ success: true, data: result?.output } as const);
const output = action.schema?.output
? action.schema.output(z).safeParse(result?.output)
: ({ success: true, data: result?.output } as const);
if (!output.success) {
throw new InputError(
`Invalid output from action "${req.params.actionId}"`,
output.error,
);
}
res.json({ output: output.data });
} catch (error) {
throw new ForwardedError(
`Failed execution of action "${req.params.actionId}"`,
error,
if (!output.success) {
throw new InputError(
`Invalid output from action "${req.params.actionId}"`,
output.error,
);
}
res.json({ output: output.data });
},
);
return router;
packages/backend-defaults/src/alpha/entrypoints/actionsRegistry/actionsRegistryServiceFactory.test.ts
+28 -5
View File
@@ -22,7 +22,7 @@ import {
import { httpRouterServiceFactory } from '../../../entrypoints/httpRouter';
import request from 'supertest';
import { actionsRegistryServiceFactory } from './actionsRegistryServiceFactory';
import { InputError } from '@backstage/errors';
import { InputError, NotFoundError } from '@backstage/errors';
import { actionsRegistryServiceRef } from '@backstage/backend-plugin-api/alpha';
describe('actionsRegistryServiceFactory', () => {
@@ -510,7 +510,7 @@ describe('actionsRegistryServiceFactory', () => {
expect(body).toMatchObject({ output: { ok: true } });
});
it('should return the error from the action if it throws', async () => {
it('should forward the original error when the action throws a known error', async () => {
const { server } = await startTestBackend({
features: [pluginSubject, ...defaultServices],
});
@@ -528,9 +528,32 @@ describe('actionsRegistryServiceFactory', () => {
expect(status).toBe(400);
expect(body).toMatchObject({
error: {
message: expect.stringContaining(
'Failed execution of action "my-plugin:test"',
),
name: 'InputError',
message: 'test',
},
});
});
it('should forward a NotFoundError from the action with 404 status', async () => {
const { server } = await startTestBackend({
features: [pluginSubject, ...defaultServices],
});
mockAction.mockRejectedValue(new NotFoundError('entity not found'));
const { body, status } = await request(server)
.post(
'/api/my-plugin/.backstage/actions/v1/actions/my-plugin:test/invoke',
)
.send({
name: 'test',
});
expect(status).toBe(404);
expect(body).toMatchObject({
error: {
name: 'NotFoundError',
message: 'entity not found',
},
});
});
packages/backend-test-utils/src/alpha/services/MockActionsRegistry.ts
+15 -22
View File
@@ -17,7 +17,7 @@ import {
BackstageCredentials,
LoggerService,
} from '@backstage/backend-plugin-api';
import { ForwardedError, InputError, NotFoundError } from '@backstage/errors';
import { InputError, NotFoundError } from '@backstage/errors';
import { JsonObject, JsonValue } from '@backstage/types';
import { z, AnyZodObject } from 'zod';
import zodToJsonSchema from 'zod-to-json-schema';
@@ -126,31 +126,24 @@ export class MockActionsRegistry
throw new InputError(`Invalid input to action "${opts.id}"`, input.error);
}
try {
const result = await action.action({
input: input.data,
credentials: opts.credentials ?? mockCredentials.none(),
logger: this.logger,
});
const result = await action.action({
input: input.data,
credentials: opts.credentials ?? mockCredentials.none(),
logger: this.logger,
});
const output = action.schema?.output
? action.schema.output(z).safeParse(result?.output)
: ({ success: true, data: result?.output } as const);
const output = action.schema?.output
? action.schema.output(z).safeParse(result?.output)
: ({ success: true, data: result?.output } as const);
if (!output.success) {
throw new InputError(
`Invalid output from action "${opts.id}"`,
output.error,
);
}
return { output: output.data };
} catch (error) {
throw new ForwardedError(
`Failed execution of action "${opts.id}"`,
error,
if (!output.success) {
throw new InputError(
`Invalid output from action "${opts.id}"`,
output.error,
);
}
return { output: output.data };
}
register<
plugins/catalog-backend/src/actions/createRegisterCatalogEntitiesAction.test.ts
+6 -5
View File
@@ -16,7 +16,6 @@
import { createRegisterCatalogEntitiesAction } from './createRegisterCatalogEntitiesAction';
import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils';
import { actionsRegistryServiceMock } from '@backstage/backend-test-utils/alpha';
import { ForwardedError } from '@backstage/errors';
describe('createRegisterCatalogEntitiesAction', () => {
it('should successfully register a catalog location with a valid URL', async () => {
@@ -78,14 +77,13 @@ describe('createRegisterCatalogEntitiesAction', () => {
).rejects.toThrow('not-a-valid-url is an invalid URL');
});
it('should throw a ForwardedError if catalog.addLocation throws an error', async () => {
it('should throw the original error if catalog.addLocation fails', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
const errorMessage = 'Failed to add location';
mockCatalog.addLocation = jest
.fn()
.mockRejectedValue(new Error(errorMessage));
.mockRejectedValue(new Error('Failed to add location'));
createRegisterCatalogEntitiesAction({
catalog: mockCatalog,
@@ -100,6 +98,9 @@ describe('createRegisterCatalogEntitiesAction', () => {
'https://github.com/example/repo/blob/main/catalog-info.yaml',
},
}),
).rejects.toThrow(ForwardedError);
).rejects.toMatchObject({
name: 'Error',
message: 'Failed to add location',
});
});
});
plugins/catalog-backend/src/actions/createUnregisterCatalogEntitiesAction.test.ts
+14 -35
View File
@@ -192,10 +192,13 @@ describe('createUnregisterCatalogEntitiesAction', () => {
);
});
it('should throw NotFoundError if no location matches the URL', async () => {
it('should throw NotFoundError with the original message if no location matches the URL', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
const locationUrl =
'https://github.com/backstage/demo/blob/master/catalog-info.yaml';
mockCatalog.getLocations = jest.fn().mockResolvedValue({
items: [
{ id: 'location-id-1', target: 'https://other-url.com/catalog.yaml' },
@@ -207,51 +210,24 @@ describe('createUnregisterCatalogEntitiesAction', () => {
actionsRegistry: mockActionsRegistry,
});
await expect(
mockActionsRegistry.invoke({
id: 'test:unregister-entity',
input: {
type: {
locationUrl:
'https://github.com/backstage/demo/blob/master/catalog-info.yaml',
},
},
}),
).rejects.toThrow(/NotFoundError/);
});
it('should throw NotFoundError with descriptive message when location not found', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
const locationUrl =
'https://github.com/backstage/demo/blob/master/catalog-info.yaml';
mockCatalog.getLocations = jest.fn().mockResolvedValue({
items: [],
});
createUnregisterCatalogEntitiesAction({
catalog: mockCatalog,
actionsRegistry: mockActionsRegistry,
});
await expect(
mockActionsRegistry.invoke({
id: 'test:unregister-entity',
input: { type: { locationUrl } },
}),
).rejects.toThrow(`Location with URL ${locationUrl} not found`);
).rejects.toMatchObject({
name: 'NotFoundError',
message: `Location with URL ${locationUrl} not found`,
});
});
it('should throw an error if catalog.getLocations throws an error', async () => {
it('should throw the original error if catalog.getLocations fails', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
const mockCatalog = catalogServiceMock();
const errorMessage = 'Failed to get locations';
mockCatalog.getLocations = jest
.fn()
.mockRejectedValue(new Error(errorMessage));
.mockRejectedValue(new Error('Failed to get locations'));
createUnregisterCatalogEntitiesAction({
catalog: mockCatalog,
@@ -268,7 +244,10 @@ describe('createUnregisterCatalogEntitiesAction', () => {
},
},
}),
).rejects.toThrow(errorMessage);
).rejects.toMatchObject({
name: 'Error',
message: 'Failed to get locations',
});
});
});
});
plugins/mcp-actions-backend/src/services/McpService.test.ts
+113
View File
@@ -26,6 +26,7 @@ import {
CallToolResultSchema,
ListToolsResultSchema,
} from '@modelcontextprotocol/sdk/types.js';
import { InputError, NotFoundError } from '@backstage/errors';
describe('McpService', () => {
it('should list the available actions as tools in the mcp backend', async () => {
@@ -343,4 +344,116 @@ describe('McpService', () => {
}),
);
});
it('should forward the original InputError when an action throws one', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
mockActionsRegistry.register({
name: 'failing-action',
title: 'Failing',
description: 'An action that throws InputError',
schema: {
input: z => z.object({ value: z.string() }),
output: z => z.object({}),
},
action: async () => {
throw new InputError('the value was invalid');
},
});
const mcpService = await McpService.create({
actions: mockActionsRegistry,
metrics: metricsServiceMock.mock(),
});
const server = mcpService.getServer({
credentials: mockCredentials.user(),
});
const client = new Client({
name: 'test client',
version: '1.0',
});
const [clientTransport, serverTransport] =
InMemoryTransport.createLinkedPair();
await Promise.all([
client.connect(clientTransport),
server.connect(serverTransport),
]);
const result = await client.request(
{
method: 'tools/call',
params: { name: 'failing-action', arguments: { value: 'test' } },
},
CallToolResultSchema,
);
expect(result).toEqual({
content: [
{
type: 'text',
text: 'InputError: the value was invalid',
},
],
isError: true,
});
});
it('should forward the original NotFoundError when an action throws one', async () => {
const mockActionsRegistry = actionsRegistryServiceMock();
mockActionsRegistry.register({
name: 'not-found-action',
title: 'Not Found',
description: 'An action that throws NotFoundError',
schema: {
input: z => z.object({ id: z.string() }),
output: z => z.object({}),
},
action: async () => {
throw new NotFoundError('entity does not exist');
},
});
const mcpService = await McpService.create({
actions: mockActionsRegistry,
metrics: metricsServiceMock.mock(),
});
const server = mcpService.getServer({
credentials: mockCredentials.user(),
});
const client = new Client({
name: 'test client',
version: '1.0',
});
const [clientTransport, serverTransport] =
InMemoryTransport.createLinkedPair();
await Promise.all([
client.connect(clientTransport),
server.connect(serverTransport),
]);
const result = await client.request(
{
method: 'tools/call',
params: { name: 'not-found-action', arguments: { id: 'abc' } },
},
CallToolResultSchema,
);
expect(result).toEqual({
content: [
{
type: 'text',
text: 'NotFoundError: entity does not exist',
},
],
isError: true,
});
});
});
plugins/mcp-actions-backend/src/services/handleErrors.test.ts
+144
View File
@@ -0,0 +1,144 @@
/*
* Copyright 2025 The Backstage Authors
*
* 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 {
InputError,
NotFoundError,
NotAllowedError,
ForwardedError,
ResponseError,
} from '@backstage/errors';
import { handleErrors } from './handleErrors';
describe('handleErrors', () => {
it('should return the error description for a known error', async () => {
const result = await handleErrors(async () => {
throw new InputError('bad input');
});
expect(result).toEqual({
content: [{ type: 'text', text: 'InputError: bad input' }],
isError: true,
});
});
it('should return the error description for a NotFoundError', async () => {
const result = await handleErrors(async () => {
throw new NotFoundError('entity not found');
});
expect(result).toEqual({
content: [{ type: 'text', text: 'NotFoundError: entity not found' }],
isError: true,
});
});
it('should return the error description for a NotAllowedError', async () => {
const result = await handleErrors(async () => {
throw new NotAllowedError('forbidden');
});
expect(result).toEqual({
content: [{ type: 'text', text: 'NotAllowedError: forbidden' }],
isError: true,
});
});
it('should rethrow an unknown error', async () => {
await expect(
handleErrors(async () => {
throw new Error('unknown problem');
}),
).rejects.toThrow('unknown problem');
});
it('should handle a ForwardedError that inherits the cause name', async () => {
const result = await handleErrors(async () => {
throw new ForwardedError(
'wrapper message',
new InputError('original error'),
);
});
// ForwardedError inherits name from cause and CustomErrorBase
// concatenates the cause message into the full message
expect(result).toEqual({
content: [
{
type: 'text',
text: 'InputError: wrapper message; caused by InputError: original error',
},
],
isError: true,
});
});
it('should extract the cause from a ResponseError', async () => {
const response = {
ok: false,
status: 400,
statusText: 'Bad Request',
headers: new Headers({ 'content-type': 'application/json' }),
text: async () =>
JSON.stringify({
error: { name: 'InputError', message: 'bad value' },
response: { statusCode: 400 },
}),
};
const responseError = await ResponseError.fromResponse(response as any);
const result = await handleErrors(async () => {
throw responseError;
});
expect(result).toEqual({
content: [{ type: 'text', text: 'InputError: bad value' }],
isError: true,
});
});
it('should recursively extract through nested ResponseErrors', async () => {
const innerResponse = {
ok: false,
status: 400,
statusText: 'Bad Request',
headers: new Headers({ 'content-type': 'application/json' }),
text: async () =>
JSON.stringify({
error: {
name: 'ResponseError',
message: 'Request failed with 400 Bad Request',
cause: { name: 'InputError', message: 'deeply nested error' },
},
response: { statusCode: 400 },
}),
};
const responseError = await ResponseError.fromResponse(
innerResponse as any,
);
const result = await handleErrors(async () => {
throw responseError;
});
expect(result).toEqual({
content: [{ type: 'text', text: 'InputError: deeply nested error' }],
isError: true,
});
});
});
plugins/mcp-actions-backend/src/services/handleErrors.ts
+3 -3
View File
@@ -35,14 +35,14 @@ const knownErrors = new Set([
'ServiceUnavailableError',
]);
// Extracts the cause error, if the provided error is `ResponseError` or
// `ForwardedError` with a cause.
// Recursively extracts the innermost cause from ResponseError or
// ForwardedError wrappers to surface the original error.
function extractCause(err: ErrorLike): ErrorLike {
if (
(err.name === 'ResponseError' || err instanceof ForwardedError) &&
isError(err.cause)
) {
return err.cause;
return extractCause(err.cause);
}
return err;
}