Skip to content

fix: do not JSON parse arbitrary bodies#627

Merged
gjtorikian merged 2 commits intomainfrom
fix-erroneous-json-parse
Apr 20, 2026
Merged

fix: do not JSON parse arbitrary bodies#627
gjtorikian merged 2 commits intomainfrom
fix-erroneous-json-parse

Conversation

@gjtorikian
Copy link
Copy Markdown
Contributor

The client should not always assume that the API response is going to be JSON.

fixes #626

@gjtorikian gjtorikian requested review from a team as code owners April 20, 2026 18:49
@gjtorikian gjtorikian requested a review from csrbarber April 20, 2026 18:49
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 20, 2026

Greptile Summary

This PR fixes a crash that occurred when the WorkOS API returns a non-JSON response body (e.g. a 202 with a plain newline) by wrapping response.json() in a try/except inside _deserialize_response. The two previously-flagged concerns — dead code and request_raw/request_list breakage — are both resolved: model is None was not added to the early-return gate, so request_raw and request_list still receive parsed JSON when available, as confirmed by the new tests.

Confidence Score: 5/5

Safe to merge; the fix is correct and prior P1 concerns are fully resolved in the current implementation.

The only remaining finding is a P2 style suggestion to narrow the except Exception clause to except ValueError. All prior P1 concerns (dead code branch, broken request_raw/request_list) are no longer applicable to the current diff.

No files require special attention.

Important Files Changed

Filename Overview
src/workos/_base_client.py Wraps response.json() in try/except to gracefully handle non-JSON bodies; logic is correct and does not break request_raw or request_list callers
tests/test_generated_client.py Adds sync and async tests for request_raw, request_list, and non-JSON 202 responses; adequately covers the new behavior
tests/test_organizations.py Updates delete_organization tests from status_code=204 to status_code=202 with non-JSON body to reflect actual API behavior

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_deserialize_response called] --> B{status 204 or no content?}
    B -->|Yes| C[return None]
    B -->|No| D[try: data = response.json]
    D -->|parse error| E[except Exception: return None]
    D -->|success| F{model is not None?}
    F -->|Yes| G[return model.from_dict data]
    F -->|No| H[return data]
    H --> I[request_raw: dict or empty dict]
    H --> J[request_list: list or raise WorkOSError]
Loading

Reviews (3): Last reviewed commit: "don't lazily shortcut" | Re-trigger Greptile

@gjtorikian
Copy link
Copy Markdown
Contributor Author

@greptile review

@gjtorikian gjtorikian merged commit 1a8ef00 into main Apr 20, 2026
10 checks passed
@gjtorikian gjtorikian deleted the fix-erroneous-json-parse branch April 20, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Bug: delete_organization raises JSONDecodeError on successful 202 response

2 participants