PDFCLOUD-5695 Expect error detail response field#40
Closed
datalogics-cgreen wants to merge 2 commits intopdfrest:mainfrom
Closed
PDFCLOUD-5695 Expect error detail response field#40datalogics-cgreen wants to merge 2 commits intopdfrest:mainfrom
datalogics-cgreen wants to merge 2 commits intopdfrest:mainfrom
Conversation
- accept both "error" and "message" when parsing pdfRest error responses - add typed error detail models (PdfRestErrorDetail/PdfRestErrorIssue) - require issue path/message fields when errorDetail is present - preserve errorDetail payload on PdfRestApiError.response_content - add sync/async client tests covering structured errorDetail responses Assisted-by: Codex
✅ Deploy Preview for pdfrest-python ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
Author
|
These changes are not required: the test failures I saw were solely due to a separate reason. Testing with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PDFCLOUD-5695
Why this change
pdfRest error responses now include richer validation data (
errorDetail.issues), but the SDK was only reading a single message field and could miss that structure entirely. In practice, callers often got a generic status-code error and had to inspect raw payloads manually to find which input field failed.This change improves developer feedback loops: invalid request payloads are now easier to diagnose, and error handling code can access structured issue details instead of relying on string parsing.
What changed (high level)
The error parsing path was updated to support both legacy and current server message keys (
messageanderror) and to model structured error details explicitly. We introduced typed models forerrorDetail/issues, exported them frompdfrest.models, and updated response extraction so structured details are preserved on raised API exceptions.The main tradeoff is intentional: we now prioritize structured payload preservation when available, which increases fidelity for callers but changes some
response_contentexpectations in existing integrations. Package version was bumped to1.1.0to reflect this behavior expansion.Behavior changes
At runtime, API errors now behave differently in a few important cases:
{"error": "..."}, the SDK now treats that as the primary human-readable message (instead of falling back to a generic status-code message).errorDetail,PdfRestApiError.response_contentnow contains the structurederrorDetailobject (includingissues) so callers can inspect field-level failures directly.Breaking change callout: there is no method-signature break, but there is an observable error-handling behavior change. Code that assumed
response_contentwas alwaysstr | Nonemay need to handledictwhen structured details are present, and code relying on previous raw-body behavior for{"error": ...}responses may need adjustment.Validation
Validation was done with focused unit coverage and local execution:
errorDetailis preserved on raisedPdfRestApiErrorand exposes expected issue paths.uv run pytest tests/test_client.py -n auto --maxschedchunk 2 -k structured_error_detail(2 passed).Limitations: full suite/matrix (
uv run pytestanduvx nox -s tests) was not run as part of this focused verification.Risks and follow-ups
Primary risk is integration compatibility around exception payload handling, especially in downstream code that pattern-matches
response_contentor stringifies exceptions for control flow.Migration note: treat
PdfRestApiError.response_contentasdict | str | None; use the exception message for user-facing text andresponse_content["issues"](when present) for programmatic diagnostics.Rollout/monitoring: watch for consumer-side type assumptions in error handlers and for support reports tied to changed exception formatting. Follow-up candidates are adding a short README note with an error-handling example and expanding integration tests for real server
errorDetailresponses.