fix: add basic error handling to manifest-server calls #772
Conversation
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@christo/manifest-server-errors#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch christo/manifest-server-errorsHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
…irbytehq/airbyte-python-cdk into christo/manifest-server-errors
|
/autofix
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds basic error handling to the manifest-server calls to improve error reporting in the connector builder UI. Previously, errors during operations like test reads were being swallowed and returned as generic 500-level errors, making debugging difficult for customers.
Key Changes
- Added try/catch blocks around all manifest-server endpoint operations (test_read, check, discover, resolve, full_resolve)
- Standardized error responses to return HTTP 400 with descriptive error messages using AirbyteTracedException
- Added comprehensive test coverage for error handling scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| airbyte_cdk/manifest_server/routers/manifest.py | Added error handling try/catch blocks to all endpoints with descriptive error messages |
| airbyte_cdk/manifest_server/api_models/manifest.py | Added ErrorResponse model for standardized error responses |
| airbyte_cdk/manifest_server/api_models/init.py | Exported the new ErrorResponse model |
| airbyte_cdk/manifest_server/openapi.yaml | Updated OpenAPI spec to include 400 error responses and version bump |
| airbyte_cdk/manifest_server/app.py | Updated application version to 0.2.0 |
| unit_tests/manifest_server/routers/test_manifest.py | Added comprehensive test coverage for error handling scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughIntroduces a standardized ErrorResponse model, updates FastAPI/OpenAPI versions to 0.2.0, adds 400 Bad Request responses across manifest endpoints, and implements centralized try/except error handling in routers to convert exceptions into structured 400 responses. Adds unit tests validating error translation for each endpoint. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant A as FastAPI Router
participant P as ManifestCommandProcessor / build_source
participant E as ErrorResponse
C->>A: POST /v1/manifest/* (payload)
rect rgb(240,245,255)
note right of A: Try core operation
A->>P: Execute command/build
P-->>A: Result or Exception
end
alt Success
A-->>C: 200 OK (normal payload)
else Exception raised
A->>A: Convert to AirbyteTracedException
A->>E: Build { detail: "...error..." }
A-->>C: 400 Bad Request (ErrorResponse)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Hi team, can we get a code review so we can ship this? This is preventing us from iterating on builder projects because errors are not exposed to the user |
2 participants
What
Resolves: https://github.com/airbytehq/airbyte-internal-issues/issues/14540
Loom Demo: https://www.loom.com/share/3f063d020c774861ab0c40a10c429b11
Adds basic try/catch error handling to the manifest-server calls to ensure errors during connector builder calls are surfaced in the builder UI.
Currently, customers are experiencing issues with errors during test reads being swallowed and thrown as generic 500-level errors.
How to Test
For quick backend tests, use Postman against a locally deployed instance of the manifest-server. This was the test manifest I used in the request body to replicate the error encountered in the Swarm ticket.
On main, this will cause an unhandled exception to throw the 500 server error.
With the changes, the actual error is caught and the message displayed in the details.
To Test in Platform
Steps to run manifest-server locally and connect to local OSS Airbyte instance:
https://gist.github.com/ChristoGrab/1abaaacc0d1879d8642c946336feb5a9
Summary by CodeRabbit