Skip to content

Comments

Add PipelineError for mcpd plugin pipeline failures#38

Merged
peteski22 merged 3 commits intomainfrom
peteski22/pipeline-plugin-error-handling
Jan 8, 2026
Merged

Add PipelineError for mcpd plugin pipeline failures#38
peteski22 merged 3 commits intomainfrom
peteski22/pipeline-plugin-error-handling

Conversation

@peteski22
Copy link
Contributor

@peteski22 peteski22 commented Jan 6, 2026

Detect pipeline failures via Mcpd-Error-Type HTTP header on 500 responses. Raises PipelineError with pipeline_flow indicating request or response failure.

  • Add PipelineError class and PIPELINE_FLOW_REQUEST/RESPONSE constants
  • Extract HTTP error handling into _raise_for_http_error() helper
  • Add comprehensive unit tests for error handling
  • Update README with error handling documentation

Summary by CodeRabbit

  • New Features

    • PipelineError exception added to the public API
    • Pipeline flow constants PIPELINE_FLOW_REQUEST and PIPELINE_FLOW_RESPONSE introduced
  • Documentation

    • Error Handling section expanded with an Exception Types table, detailed usage examples and guidance on exception chaining
  • Tests

    • New tests covering PipelineError behaviour, flow mapping, header handling and HTTP error routing

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

Detect pipeline failures via Mcpd-Error-Type HTTP header on 500 responses.
Raises PipelineError with pipeline_flow indicating request or response failure.

- Add PipelineError class and PIPELINE_FLOW_REQUEST/RESPONSE constants
- Extract HTTP error handling into _raise_for_http_error() helper
- Add comprehensive unit tests for error handling
- Update README with error handling documentation
@peteski22 peteski22 added the enhancement New feature or request label Jan 6, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 6, 2026

Walkthrough

Adds pipeline flow constants and a new PipelineError exception, centralises HTTP error handling into a private helper in the client, updates public exports, introduces pipeline-aware error raising, and expands README error-handling docs and unit tests.

Changes

Cohort / File(s) Summary
Documentation
README.md
Enhanced Error Handling section: clarified SDK error semantics, added an Exception Types table, expanded Example usage and structured snippets.
Package exports
src/mcpd/__init__.py
Exports added: PipelineError, PIPELINE_FLOW_REQUEST, PIPELINE_FLOW_RESPONSE.
Exceptions
src/mcpd/exceptions.py
Added PIPELINE_FLOW_REQUEST, PIPELINE_FLOW_RESPONSE, _PIPELINE_ERROR_FLOWS mapping and a PipelineError class. Note: the file contains a duplicate/second PipelineError definition (identical signature).
Client error handling
src/mcpd/mcpd_client.py
Introduced private _raise_for_http_error and _MCPD_ERROR_TYPE_HEADER, consolidated HTTP→MCPD exception mapping, and raise PipelineError for 500 responses with pipeline error headers.
Tests — exceptions
tests/unit/test_exceptions.py
Added TestPipelineError suite validating PipelineError attributes, flow constants, header→flow mapping (case‑insensitive) and integration with client error paths.
Tests — client
tests/unit/test_mcpd_client.py
Added tests for _raise_for_http_error covering authentication, not-found, pipeline (request/response) failures, generic tool-execution errors, empty-body messaging and exception chaining.

Suggested reviewers

  • aittalam
  • dpoulopoulos
🚥 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 pull request title accurately summarises the main change: adding a PipelineError exception class to handle mcpd plugin pipeline failures, which is the primary objective of this changeset.
Docstring Coverage ✅ Passed Docstring coverage is 93.75% which is sufficient. The required threshold is 80.00%.

✏️ 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 025238f and 3b97911.

📒 Files selected for processing (1)
  • src/mcpd/mcpd_client.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/mcpd/mcpd_client.py (1)
src/mcpd/exceptions.py (5)
  • ConnectionError (82-100)
  • McpdError (28-79)
  • PipelineError (328-379)
  • ServerNotFoundError (125-152)
  • ToolExecutionError (224-260)
🪛 Ruff (0.14.10)
src/mcpd/mcpd_client.py

52-54: Avoid specifying long messages outside the exception class

(TRY003)


57-57: Avoid specifying long messages outside the exception class

(TRY003)


74-78: Avoid specifying long messages outside the exception class

(TRY003)


81-85: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (4)
src/mcpd/mcpd_client.py (4)

16-16: LGTM! Import additions are appropriate.

The new imports support the pipeline error handling feature correctly:

  • NoReturn for the error-raising helper function signature
  • _PIPELINE_ERROR_FLOWS for header-to-flow mapping
  • PipelineError for the new exception type

Also applies to: 24-24, 28-29


39-40: LGTM! Good use of a named constant.

Declaring the header name as a private constant improves maintainability and avoids magic strings in the code.


43-86: LGTM! Excellent error handling consolidation with robust defensive patterns.

This helper function centralises HTTP error handling effectively. Key strengths:

  • Defensive header extraction (line 61): The pattern (error.response.headers.get(_MCPD_ERROR_TYPE_HEADER) or "").lower() correctly handles the case where the header is absent (returns None), preventing potential errors.
  • Correct logic flow: Status codes are checked in the right order (401 → 404 → 500 with header → other 5xx → 4xx), ensuring pipeline errors are properly differentiated from generic server errors.
  • Proper exception chaining: All exceptions use from error to preserve the original HTTP error context for debugging.
  • Safe fallbacks: Line 64 handles None response text gracefully.

The static analysis TRY003 hints can be safely ignored—the error messages are context-specific and appropriately parameterised for each scenario.


269-269: LGTM! Clean refactoring improves maintainability.

Replacing inline error handling with a call to _raise_for_http_error centralises the error mapping logic whilst preserving the existing error semantics. This makes the codebase easier to maintain and test.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI Agents
In @src/mcpd/exceptions.py:
- Around line 361-379: The __init__ signature of PipelineError has parameters
with default None but lacks explicit union typing; update the annotations to use
Optional[str] (or Union[str, None]) for server_name, operation, and
pipeline_flow in PipelineError.__init__, and mirror this explicit Optional
typing for other exception constructors in the file that use = None (e.g.,
ServerNotFoundError, ToolExecutionError) to keep signatures consistent with PEP
484 and the rest of the module.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0624c3 and d6a723e.

📒 Files selected for processing (6)
  • README.md
  • src/mcpd/__init__.py
  • src/mcpd/exceptions.py
  • src/mcpd/mcpd_client.py
  • tests/unit/test_exceptions.py
  • tests/unit/test_mcpd_client.py
🧰 Additional context used
🧬 Code graph analysis (3)
src/mcpd/mcpd_client.py (1)
src/mcpd/exceptions.py (4)
  • McpdError (28-79)
  • PipelineError (328-379)
  • ServerNotFoundError (125-152)
  • ToolExecutionError (224-260)
src/mcpd/__init__.py (1)
src/mcpd/exceptions.py (4)
  • AuthenticationError (103-122)
  • ConnectionError (82-100)
  • McpdError (28-79)
  • PipelineError (328-379)
tests/unit/test_mcpd_client.py (2)
src/mcpd/exceptions.py (6)
  • AuthenticationError (103-122)
  • McpdError (28-79)
  • PipelineError (328-379)
  • ServerNotFoundError (125-152)
  • ServerUnhealthyError (155-187)
  • ToolExecutionError (224-260)
src/mcpd/mcpd_client.py (3)
  • HealthStatus (102-118)
  • McpdClient (121-1054)
  • _raise_for_http_error (43-85)
🪛 Ruff (0.14.10)
src/mcpd/mcpd_client.py

52-54: Avoid specifying long messages outside the exception class

(TRY003)


57-57: Avoid specifying long messages outside the exception class

(TRY003)


74-78: Avoid specifying long messages outside the exception class

(TRY003)


81-85: Avoid specifying long messages outside the exception class

(TRY003)

src/mcpd/exceptions.py

361-361: Missing return type annotation for special method __init__

Add return type annotation: None

(ANN204)


364-364: PEP 484 prohibits implicit Optional

Convert to T | None

(RUF013)


365-365: PEP 484 prohibits implicit Optional

Convert to T | None

(RUF013)


366-366: PEP 484 prohibits implicit Optional

Convert to T | None

(RUF013)

🔇 Additional comments (7)
src/mcpd/__init__.py (1)

16-42: LGTM!

The new exports for PipelineError, PIPELINE_FLOW_REQUEST, and PIPELINE_FLOW_RESPONSE are correctly imported and re-exported. The additions follow the existing patterns in the file and properly expose the new public API surface.

tests/unit/test_mcpd_client.py (1)

44-193: Comprehensive test coverage for _raise_for_http_error.

The test class provides thorough coverage of the new helper function:

  • All HTTP status code branches (401, 404, 500 with/without headers, 5xx, 4xx)
  • Pipeline flow detection for both request and response failures
  • Edge cases including case-insensitive header handling, empty response bodies, and error chaining

The helper method _make_http_error is a clean abstraction for creating mock HTTP errors.

README.md (1)

283-330: Well-documented error handling section.

The expanded documentation provides:

  • Clear exception hierarchy table covering all SDK exceptions
  • Practical example demonstrating PipelineError handling with flow constants
  • Proper exception ordering (specific before generic)

The example correctly shows how to differentiate between request and response pipeline failures using the exported constants.

src/mcpd/mcpd_client.py (2)

43-85: Well-structured HTTP error handling helper.

The function correctly centralises HTTP error mapping logic with proper exception chaining. The flow handles:

  1. Authentication failures (401)
  2. Not found errors (404)
  3. Pipeline failures (500 with specific header)
  4. Generic server errors (5xx)
  5. Client errors (4xx)

The case-insensitive header lookup via .lower() ensures robustness against header value casing variations.


268-269: Clean refactoring of HTTP error handling.

The integration delegates all HTTP error classification to the new _raise_for_http_error helper, simplifying _perform_call whilst maintaining the same exception semantics.

tests/unit/test_exceptions.py (1)

464-610: Thorough test coverage for PipelineError.

The test class provides comprehensive coverage:

  • Unit tests for exception attributes and constants
  • Verification of the internal _PIPELINE_ERROR_FLOWS mapping
  • Integration tests exercising the full error path through _perform_call
  • Edge cases including case-insensitive header handling and empty response bodies

The tests correctly mirror the tests in test_mcpd_client.py whilst focusing on exception-specific behaviour.

src/mcpd/exceptions.py (1)

13-25: Well-defined pipeline flow constants and mapping.

The constants and internal mapping are correctly structured:

  • Flow constants have clear docstring comments explaining their semantics
  • The _PIPELINE_ERROR_FLOWS mapping uses lowercase keys, aligning with the case-insensitive lookup in _raise_for_http_error

Handle potential None header value when checking for pipeline error type.
@peteski22 peteski22 merged commit 7fde7d6 into main Jan 8, 2026
7 of 8 checks passed
@peteski22 peteski22 deleted the peteski22/pipeline-plugin-error-handling branch January 8, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants