feat: raise ControlPlaneError on API error responses instead of returning Error#139
feat: raise ControlPlaneError on API error responses instead of returning Error#139devin-ai-integration[bot] wants to merge 3 commits into
Conversation
…ning Error BREAKING CHANGE: Control-plane API functions now raise ControlPlaneError on 4xx/5xx responses instead of returning Union[Model, Error]. - Add BlaxelAPIError base class in src/blaxel/core/errors.py - Add ControlPlaneError(BlaxelAPIError) in client/errors.py - Add raise_on_error=True flag to Client (opt-out with False) - Update Jinja template to raise on Error responses - Transform all 35 generated API files to raise ControlPlaneError - Make SandboxAPIError, DriveAPIError, VolumeAPIError extend BlaxelAPIError - Wrap callers in try/except ControlPlaneError (drive, volume, sandbox, tools) - Export BlaxelAPIError and ControlPlaneError from blaxel.core - Add MIGRATION.md with upgrade instructions Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Ensures update_metadata, update_ttl, update_lifecycle, create, and delete all translate ControlPlaneError into SandboxAPIError, consistent with the get() method. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Needs attention
The follow-up commit correctly wraps the remaining sandbox API calls. One inconsistency: src/blaxel/core/sandbox/sync/sandbox.py:458 — the sync _delete_sandbox_by_name is missing the if response is None: raise ValueError(...) guard that exists in the async version at sandbox/default/sandbox.py:535. With raise_on_error=False, the sync version silently returns None instead of raising, causing a type mismatch for callers. Add the same None check before return response.
Tag @mendral-app with feedback or questions. View session
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@mendral-app Good catch — added the |
Summary
Control-plane API functions now raise
ControlPlaneErroron 4xx/5xx responses instead of returningUnion[Model, Error]. This aligns the generated client with the Go SDK and the existing Python domain wrappers (SandboxAPIError,DriveAPIError,VolumeAPIError).Changes
New unified error hierarchy:
src/blaxel/core/errors.py— NewBlaxelAPIErrorbase exception withmessage,status_code,error_code, andresponseattributes.src/blaxel/core/client/errors.py— NewControlPlaneError(BlaxelAPIError)that wraps theErrordata model and preserves it aserror_model.src/blaxel/core/client/client.py— Addedraise_on_error=Trueflag (opt-out withFalsefor backward compat).templates/endpoint_module.py.jinja— Updated template to raiseControlPlaneErrorwhenclient.raise_on_errorisTrueand response type isError._parse_responsefunctions now conditionally raiseControlPlaneErrorinstead of returningErrormodels.SandboxAPIError,DriveAPIError,VolumeAPIErrornow extendBlaxelAPIError.drive.py,volume.py,sandbox/default/sandbox.py,sandbox/sync/sandbox.pyupdated to catchControlPlaneErrorand re-raise as domain-specific errors, withisinstance(response, Error)fallback forraise_on_error=False.BlaxelAPIErrorandControlPlaneErrorexported fromblaxel.core.MIGRATION.md— Migration guide with before/after code examples and opt-out instructions.Review & Testing Checklist for Human
ControlPlaneErroris raised with correctstatus_code,error_code, anderror_modelraise_on_error=Falseand confirm the oldUnion[Model, Error]return pattern still worksDriveInstance.create(),VolumeInstance.create(),SandboxInstance.get()still raise their domain-specific errors (which now extendBlaxelAPIError)BlaxelAPIErrorcatch-all: Confirmexcept BlaxelAPIErrorcatchesControlPlaneError,SandboxAPIError,DriveAPIError, andVolumeAPIErrormake sdk-controlplane) and verify the template produces correct output withraise_on_errorgatingNotes
test_update_lifecycle_with_different_policy_types_preserves_filessends duplicateTTL_IDLEexpiration policies (the same policy appears twice in the list), which the API correctly rejects with a 409. Previously this error was silently swallowed becauseErrorwas returned as a data model and the test accidentally continued without checking it. Now the error is properly surfaced asSandboxAPIError. The test needs to be fixed separately by removing the duplicate policy entry.make sdk-controlplaneregeneration, these files will be overwritten with template-generated versions.isinstance(response, Error)checks in wrapper code are kept as fallback forraise_on_error=Falseusers — they are unreachable in the defaultraise_on_error=Truepath.Link to Devin session: https://app.devin.ai/sessions/cbe4c13c6e344b8e90d869bc8af2b56b
Requested by: @Joffref
Note
This PR introduces a unified exception hierarchy for Blaxel API errors. Control-plane API functions now raise
ControlPlaneError(extendingBlaxelAPIError) on 4xx/5xx responses instead of returningUnion[Model, Error]. Araise_on_error=Trueflag onClientallows opt-out. Domain wrappers (drive,volume,sandbox) are updated to catchControlPlaneErrorand re-raise as their domain-specific errors, which now also extendBlaxelAPIError. All 35 generated API files and the Jinja template are updated consistently.Written by Mendral for commit 7c3b72e.