Skip to content

fix: preserve HTTPException status in update_docling_preset (#1586)#2004

Open
hunterxtang wants to merge 3 commits into
mainfrom
fix/1586-docling-preset-status-main
Open

fix: preserve HTTPException status in update_docling_preset (#1586)#2004
hunterxtang wants to merge 3 commits into
mainfrom
fix/1586-docling-preset-status-main

Conversation

@hunterxtang

@hunterxtang hunterxtang commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1586. Re-lands the fix from #1922 (merged into release-0.5.1) on a branch pushed directly to this repo, so CI has access to the secrets forked PRs can't use — see #1814 for why the forked version's E2E checks could never pass. Applied by hand rather than cherry-picked because src/api/settings.py has since been split into the src/api/settings/ package on main; the endpoint change now lives in src/api/settings/endpoints.py. Includes a unit regression test asserting an invalid preset returns 400, not 500.

Summary by CodeRabbit

  • Bug Fixes
    • Preserves the original error status when an invalid preset is submitted, so bad input continues to return a 400 response.
    • Unexpected failures now return a consistent 500 response with a clearer, fixed message instead of exposing internal error details.
  • Tests
    • Added coverage for invalid preset handling to confirm the correct error code and message are returned.

@github-actions github-actions Bot added backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) tests bug 🔴 Something isn't working. labels Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4cfa4647-215a-40b8-b050-58c7e66936e9

📥 Commits

Reviewing files that changed from the base of the PR and between 4f104ec and 40334e1.

📒 Files selected for processing (2)
  • src/api/settings/endpoints.py
  • tests/unit/api/test_settings_endpoints.py

Walkthrough

Exception handling in update_docling_preset was changed so HTTPException is re-raised unchanged, preserving its original status code, while non-HTTP exceptions are logged and converted to a generic 500 error. A new unit test validates that invalid presets return a 400 status.

Changes

HTTPException preservation fix

Layer / File(s) Summary
Exception handling fix and test
src/api/settings/endpoints.py, tests/unit/api/test_settings_endpoints.py
update_docling_preset re-raises HTTPException unchanged and returns a fixed-detail 500 for other exceptions after logging; a new async test verifies an invalid preset raises HTTPException with status 400 and detail mentioning "Invalid preset" and the preset value.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant update_docling_preset
  participant Logger

  Client->>update_docling_preset: request with invalid preset
  update_docling_preset->>update_docling_preset: raise HTTPException(400)
  update_docling_preset->>update_docling_preset: catch HTTPException, re-raise
  update_docling_preset-->>Client: HTTPException(400)

  Client->>update_docling_preset: request causing generic exception
  update_docling_preset->>Logger: log error
  update_docling_preset-->>Client: HTTPException(500, fixed detail)
Loading

Related issues: #1586 - Fixes the bug where HTTPException(400) was swallowed and re-raised as a 500 error in update_docling_preset.

Suggested labels: bug, tests

Suggested reviewers: edwinjosechittilappilly

🐰 A preset gone wrong, once lost in the fray,
Now wears its true four-hundred, come what may,
A test stands guard against the five-hundred veil,
Ensuring no mistake will hide the trail. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main fix: preserving HTTPException status in update_docling_preset.
Linked Issues check ✅ Passed The code preserves HTTPException re-raising and adds regression coverage for the 400 invalid-preset case.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to the endpoint fix and its unit test, with no unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1586-docling-preset-status-main

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.

@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jul 2, 2026
@github-actions github-actions Bot added the lgtm label Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. lgtm tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: HTTPException(400) swallowed as 500 in update_docling_preset

2 participants