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

Signed-off-by: benjdlambert <ben@blam.sh>

plugins/mcp-actions-backend/src/services/McpService.test.ts

@@ -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

@@ -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

@@ -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;
 }