Add PipelineError for plugin pipeline failures #22

peteski22 · 2026-01-06T19:18:29Z

Adds support for detecting and handling mcpd plugin pipeline failures:

Add PipelineError class with pipelineFlow, serverName, and operation properties
Detect Mcpd-Error-Type header on 500 responses (request-pipeline-failure, response-pipeline-failure)
Enrich PipelineError with server/tool context when called through performCall
Export PipelineFlow type and PIPELINE_FLOW_REQUEST/PIPELINE_FLOW_RESPONSE constants
Add comprehensive tests for pipeline error handling
Update README with PipelineError documentation

Summary by CodeRabbit

New Features
- New public PipelineError type and pipeline flow constants to surface pipeline failures.
- Richer error diagnostics that preserve pipeline flow and attach server/operation context.
- Existing error behaviours retained as fallback for non-pipeline failures.
Documentation
- README updated with PipelineError semantics and examples.
Tests
- Expanded tests covering pipeline error detection, flows and context propagation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Adds support for detecting and handling mcpd plugin pipeline failures: - Add PipelineError class with pipelineFlow, serverName, and operation properties - Detect Mcpd-Error-Type header on 500 responses (request-pipeline-failure, response-pipeline-failure) - Enrich PipelineError with server/tool context when called through performCall - Export PipelineFlow type and PIPELINE_FLOW_REQUEST/PIPELINE_FLOW_RESPONSE constants - Add comprehensive tests for pipeline error handling - Update README with PipelineError documentation

coderabbitai · 2026-01-06T19:18:38Z

Walkthrough

The PR introduces a new PipelineError class with pipeline flow information to handle MCPD daemon pipeline failures. The error is defined in the errors module, exported from the main index, implemented in the client to detect 500 responses with pipeline failure headers, and covered by new unit tests.

Changes

Cohort / File(s)	Summary
Error types and exports `src/errors.ts`, `src/index.ts`	Introduces `PipelineError` class extending `McpdError` with `serverName`, `operation`, and `pipelineFlow`. Adds `PIPELINE_FLOW_REQUEST`, `PIPELINE_FLOW_RESPONSE`, and `PipelineFlow` type. Re-exported from main index.
Client error handling `src/client.ts`	Adds pipeline-aware detection for 500 responses using `Mcpd-Error-Type` header and `PIPELINE_ERROR_FLOWS` mapping; throws `PipelineError` with appropriate flow. Enriches and rethrows `PipelineError` with server/tool context in catch and tool paths.
Documentation `README.md`	Adds a dedicated `PipelineError` section describing semantics, properties and usage examples.
Tests `tests/unit/client.test.ts`	Adds "PipelineError handling" suite covering response/request pipeline failures, header case-insensitivity, enrichment with `serverName`/`operation`, and distinction from generic `McpdError`.

Suggested reviewers

agpituk
khaledosman

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add PipelineError for plugin pipeline failures' accurately summarises the main change—introducing a new PipelineError class to handle pipeline failures. It directly aligns with the primary objective of the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8ed00c and 785921f.

📒 Files selected for processing (1)

tests/unit/client.test.ts

🧰 Additional context used

🧬 Code graph analysis (1)

tests/unit/client.test.ts (2)

src/index.ts (2)

PipelineError (25-25)

McpdError (22-22)

src/errors.ts (2)

PipelineError (272-291)

McpdError (34-41)

🔇 Additional comments (2)

tests/unit/client.test.ts (2)

3-8: LGTM: Imports correctly updated for pipeline error testing.

The addition of McpdError and PipelineError imports aligns with the new test suite requirements.

1295-1436: Excellent test coverage for PipelineError handling.

The test suite comprehensively covers pipeline error scenarios with well-structured tests:

Response and request pipeline failures with correct pipelineFlow values

Fallback to McpdError when the header is absent

Case-insensitive header handling

Context enrichment with serverName and operation in performCall

Proper distinction between pipeline and non-pipeline 500 errors

The .catch((e: unknown) => e) pattern elegantly avoids the need for fail assertions in try/catch blocks, addressing previous review concerns.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI Agents

In @tests/unit/client.test.ts:
- Around line 1296-1327: The test duplicates the same mock response and lacks a
fail assertion inside the try/catch; consolidate to a single
mockFetch.mockResolvedValueOnce(...) before the assertions, call await
expect(client.listServers()).rejects.toThrow(PipelineError) and then if you
still want to inspect the error instance use a second await
client.listServers().catch(error => {
expect(error).toBeInstanceOf(PipelineError); const pipelineError = error as
PipelineError; expect(pipelineError.pipelineFlow).toBe("response");
expect(pipelineError.message).toContain("Response processing failed"); }) but
ensure you add an explicit fail() (or another expect to reject) immediately
after the awaited call in the try block so the test fails if no error is thrown;
reference client.listServers, PipelineError, and mockFetch in the changes.
- Around line 1329-1457: Tests that expect an exception (the try/catch blocks
around client.listServers() and client.servers.time!.tools.get_current_time!())
currently lack a failing assertion if no error is thrown; add an immediate
assertion like expect.fail("Expected error to be thrown") right after each
awaited call inside those try blocks (i.e., after await client.listServers() in
the request/500/case-insensitive/other-500 tests and after await
client.servers.time!.tools.get_current_time!({ timezone: "UTC" }) in the
performCall test) so the test fails if no error occurs.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 415721a and a8ed00c.

📒 Files selected for processing (5)

README.md
src/client.ts
src/errors.ts
src/index.ts
tests/unit/client.test.ts

🧰 Additional context used

🧬 Code graph analysis (3)

tests/unit/client.test.ts (1)

src/errors.ts (2)

PipelineError (272-291)

McpdError (34-41)

src/client.ts (1)

src/errors.ts (4)

PipelineFlow (23-25)

PIPELINE_FLOW_REQUEST (13-13)

PIPELINE_FLOW_RESPONSE (18-18)

PipelineError (272-291)

src/errors.ts (2)

src/index.ts (5)

PIPELINE_FLOW_REQUEST (26-26)

PIPELINE_FLOW_RESPONSE (27-27)

PipelineFlow (34-34)

PipelineError (25-25)

McpdError (22-22)

src/client.ts (8)

serverName (386-401)

serverName (424-447)

serverName (470-489)

serverName (512-535)

serverName (558-581)

serverName (604-614)

serverName (736-753)

serverName (817-866)

🔇 Additional comments (10)

src/index.ts (1)

25-34: LGTM!

The new pipeline-related exports are correctly added and follow the existing re-export pattern. The PipelineFlow type is properly exported with the type keyword for type-only exports.

README.md (1)

612-650: LGTM!

The documentation comprehensively covers the new PipelineError class, including:

Clear explanation of when pipeline errors occur

Distinction between request and response pipeline failures

Complete property documentation

Practical example showing how to handle different pipeline flows

This provides excellent guidance for SDK users.

tests/unit/client.test.ts (2)

3-8: LGTM!

Imports correctly updated to include McpdError and PipelineError from the errors module.

1387-1438: LGTM!

This test thoroughly validates the error enrichment logic, correctly asserting that serverName and operation are populated when a PipelineError propagates through performCall. The mock sequence properly simulates the health check, tools list, and tool call chain.

src/errors.ts (2)

10-25: LGTM!

The pipeline flow constants and type are well-designed:

Using as const ensures literal type inference

The PipelineFlow union type correctly references the constants, providing type safety without string duplication

221-291: LGTM!

The PipelineError class is well-implemented:

Follows the established pattern of other error classes in this module

Comprehensive JSDoc documentation with clear explanations of request vs response pipeline failures

Includes a practical code example in the documentation

Correctly calls Error.captureStackTrace for proper stack traces

src/client.ts (4)

22-25: LGTM!

Imports correctly added for the new pipeline error handling functionality.

81-92: LGTM!

The header constant and flow mapping are well-designed:

PIPELINE_ERROR_FLOWS uses lowercase keys to support case-insensitive header value matching

The mapping cleanly translates HTTP header values to typed flow constants

256-274: LGTM!

The pipeline error detection logic is sound:

Correctly checks for 500 status before examining the header

Converts header value to lowercase for case-insensitive matching

Falls back gracefully through errorModel?.detail, body, and finally a default message

Leaves serverName and operation undefined for caller enrichment

843-852: LGTM!

The error enrichment in #performCall correctly:

Catches PipelineError and re-throws with server/tool context

Preserves the original pipelineFlow and cause from the caught error

Formats operation as "serverName.toolName" for clear error context

tests/unit/client.test.ts

peteski22 added the enhancement New feature or request label Jan 6, 2026

coderabbitai bot reviewed Jan 6, 2026

View reviewed changes

tests/unit/client.test.ts Show resolved Hide resolved

tests/unit/client.test.ts Show resolved Hide resolved

PR feedback addressed

785921f

peteski22 requested a review from khaledosman January 7, 2026 13:48

khaledosman approved these changes Jan 8, 2026

View reviewed changes

peteski22 merged commit c5cadde into main Jan 8, 2026
3 checks passed

peteski22 deleted the peteski22/pipeline-plugin-error-handling branch January 8, 2026 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add PipelineError for plugin pipeline failures #22

Add PipelineError for plugin pipeline failures #22

Uh oh!

peteski22 commented Jan 6, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 6, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add PipelineError for plugin pipeline failures #22

Add PipelineError for plugin pipeline failures #22

Uh oh!

Conversation

peteski22 commented Jan 6, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peteski22 commented Jan 6, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 6, 2026 •

edited

Loading