fix: preserve HTTPException status in update_docling_preset (#1586) by hunterxtang · Pull Request #1814 · langflow-ai/openrag

hunterxtang · 2026-06-09T16:43:21Z

Summary by CodeRabbit

Bug Fixes
- Improved error handling for the docling-preset endpoint: client-side issues (such as invalid preset values) now preserve the original HTTP status codes and messages instead of being converted into a generic server error.
Tests
- Added a unit test to confirm invalid preset requests return a 400 response with the expected error detail.

coderabbitai · 2026-06-09T16:43:36Z

Walkthrough

The deprecated /settings/docling-preset endpoint now re-raises caught HTTPException instances in update_docling_preset, preserving original HTTP status codes; other exceptions are logged and result in a generic 500 HTTPException. A unit test asserting a 400 for an invalid preset was added.

Changes

HTTP Exception Handling Fix

Layer / File(s)	Summary
Preserve HTTPException and add unit test `src/api/settings/endpoints.py`, `tests/unit/api/test_settings_endpoints.py`	`update_docling_preset` now explicitly catches and re-raises `fastapi.HTTPException` before a generic `except Exception` that logs and raises a 500; a pytest-asyncio unit test asserts a 400 response for an invalid preset input.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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 directly describes the main fix: preserving HTTPException status in the update_docling_preset function, which is the core change in this pull request.
Linked Issues check	✅ Passed	The pull request implements all key objectives from issue `#1586`: adds specific HTTPException handler to preserve 400 status codes, removes raw exception text from client responses, retains it in logs, maintains exception chaining, and includes regression test.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the HTTPException handling in update_docling_preset and adding regression test coverage; no unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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 and usage tips.}

…-ai#1586)

Vchen7629

LGTM

mpawlow

Good job @hunterxtang

Code Review 1

See PR comments: (1a), (1b) for relative easy tweaks / fixes

mpawlow · 2026-06-11T22:18:19Z

(1a) [Normal] Raw exception message leaked in 500 response detail string

Problem

Embedding str(e) in API response bodies can expose internal implementation details (file paths, library internals, config values, stack-frame fragments) to any caller — a recognized security anti-pattern (OWASP A05:2021 — Security Misconfiguration; CWE-209: Information Exposure Through an Error Message).

The error is already fully captured server-side via logger.error("Failed to update docling settings", error=str(e)), so no diagnostic value is lost by removing str(e) from the detail.

Note: This was explicitly called out in the linked issue's own proposed fix (bug: HTTPException(400) swallowed as 500 in update_docling_preset #1586):
- status_code=500, detail=f"Failed to update docling settings: {str(e)}" + status_code=500, detail="Failed to update docling settings"
The PR applied the except HTTPException: raise part of that fix but did not carry forward this secondary hardening.

Code References

src/api/settings/endpoints.py:1630 — primary (this PR's scope)

Potential Solution

Strip the raw exception from the client-facing detail string:

except Exception as e: logger.error("Failed to update docling settings", error=str(e)) raise HTTPException( status_code=500, detail="Failed to update docling settings" ) from e

mpawlow · 2026-06-11T22:24:37Z

-
+    except HTTPException:
+        # Preserve intended HTTP status codes (e.g. 400 for an invalid preset)
+        raise


(1b) [Minor] No regression test added to guard the fixed behavior

Problem

The PR fixes a bug where an invalid preset silently produced a 500 instead of the intended 400, but no test was added to assert the corrected behavior.

Without a test, any future refactor that reorders or widens the except chain could silently reproduce the original bug with no CI signal.

A unit test calling update_docling_preset directly with an invalid preset body and asserting status_code=400 would require no infra and would be straightforward to add.

Code References

src/api/settings/endpoints.py:1590–1594 — the raise HTTPException(status_code=400, ...) path the fix protects

tests/unit/test_docling_service.py — closest existing test file; an additional test belongs here or in a new test_settings_endpoints.py

Potential Solution

import pytest from fastapi import HTTPException from unittest.mock import AsyncMock @pytest.mark.asyncio async def test_update_docling_preset_invalid_preset_returns_400(): from api.settings.endpoints import update_docling_preset from api.settings.models import DoclingPresetBody body = DoclingPresetBody(preset="nonexistent_preset") with pytest.raises(HTTPException) as exc_info: await update_docling_preset(body=body, session_manager=AsyncMock(), user=AsyncMock()) assert exc_info.value.status_code == 400

…gression test (langflow-ai#1586)

hunterxtang · 2026-06-12T16:56:24Z

1a) Removed str(e) from the client-facing detail so the 500 no longer leaks internal exception text; kept str(e) in the logger.error call for server-side diagnostics, and preserved the from e chaining. The detail is now a static "Failed to update docling settings".

1b) Added test_update_docling_preset_invalid_preset_returns_400 in tests/unit/api/test_settings_endpoints.py. It sends an invalid preset and asserts the call raises HTTPException with status_code == 400. To confirm it actually guards the fix, I verified it fails (returns 500) when the except HTTPException: raise block is removed, then restored the block. Any future refactor that reorders or widens the except chain will trip CI, which was the concern here.

mpawlow · 2026-06-12T17:08:42Z

Thanks @hunterxtang

Changes look good 👍
Running remaining workflows...
- Waiting to see if E2E Tests / Integration Tests pass...

mpawlow · 2026-06-12T17:31:45Z

Error: OPENAI_API_KEY is not set

Integration tests are failing
I think this is a workflow error related to this being a forked Pull Request
Following up with the rest of the team on this...

mpawlow · 2026-06-12T17:53:46Z

Error: 6-12T17:26:14,233][ERROR][o.o.s.a.BackendRegistry ] [504d3485a6c9] OpenSearch Security not initialized. (you may need to run securityadmin)

Error: OPENAI_API_KEY is not set

❌ Suspicion: Forked PR causing the E2E tests and integration tests to fail

CC @Wallgau @lucaseduoli

mpawlow

Code Review 2

✅ Approved / LGTM 🚀
⚠️ Need to resolve E2E / Integration test failures in the workflow before merge (potential infrastructure problems)

coderabbitai

🧹 Nitpick comments (1)

src/api/settings/endpoints.py (1)
1085-1120: ⚖️ Poor tradeoff

Unrelated refactoring mixed with the docling preset fix.

The PR objectives (issue #1586) describe only the HTTPException status preservation fix in update_docling_preset (lines 1600-1605). However, this diff also includes an unrelated refactoring of OpenSearch client creation in the onboarding function (lines 1085-1120). The AI summary mentions this refactoring but the PR objectives and issue description do not.

Mixing unrelated changes in a single PR reduces reviewability and makes rollback more difficult if one change needs to be reverted independently. Consider moving the OpenSearch client refactoring to a separate PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/settings/endpoints.py` around lines 1085 - 1120, Remove the
OpenSearch client creation refactoring from this PR as it is unrelated to the
main objective of fixing HTTPException status preservation in the
update_docling_preset function. The refactoring of admin_username determination
and the associated logging in the onboarding function (lines 1085-1120) should
be moved to a separate pull request to maintain focus on the issue `#1586` fix and
improve code review clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/api/settings/endpoints.py`:
- Around line 1085-1120: Remove the OpenSearch client creation refactoring from
this PR as it is unrelated to the main objective of fixing HTTPException status
preservation in the update_docling_preset function. The refactoring of
admin_username determination and the associated logging in the onboarding
function (lines 1085-1120) should be moved to a separate pull request to
maintain focus on the issue `#1586` fix and improve code review clarity.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 381ed3c7-f2f6-4781-84df-ce38f9d3ee75

📥 Commits

Reviewing files that changed from the base of the PR and between 6b457b8 and c2b0984.

📒 Files selected for processing (1)

src/api/settings/endpoints.py

mpawlow · 2026-06-16T13:21:33Z

🚧 Re-running the GitHub workflow to see if the E2E / Integration test failures are resolved

mpawlow · 2026-06-16T14:06:04Z

Still failing...

This has got to be the cause --> fix: preserve HTTPException status in update_docling_preset (#1586) #1814 (comment)

@Wallgau @lucaseduoli

Question: Should we just forcibly merge this forked PR in the interim OR attempt to resolve the underlying problem of forked PR access to the OpenAI credentials within the GitHub Action context?

Wallgau · 2026-06-16T14:13:38Z

Still failing...

This has got to be the cause --> fix: preserve HTTPException status in update_docling_preset (#1586) #1814 (comment)

@Wallgau @lucaseduoli

Question: Should we just forcibly merge this forked PR in the interim OR attempt to resolve the underlying problem of forked PR access to the OpenAI credentials within the GitHub Action context?

we should probably create a new branch from the release one and apply changes there, @lucaseduoli ?

mpawlow · 2026-06-16T14:57:35Z

Anyone with collaborator access to this repository can use these secrets and variables for actions. They are not passed to workflows that are triggered by a pull request from a fork.

See: https://github.com/langflow-ai/openrag/settings/secrets/actions
There is supposed to be a setting to override this in Settings > Actions > General > Fork pull request workflows
- Then enable a Send secrets to workflows from pull requests option
However, I don't see it. It is likely defined at the langflow GitHub org level (which I don't have admin access to)
- https://github.com/organizations/langflow-ai/settings

github-actions Bot added community backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. labels Jun 9, 2026

fix: preserve HTTPException status in update_docling_preset (langflow…

cd3713f

…-ai#1586)

hunterxtang force-pushed the fix/1586-docling-preset-status branch from e3de2b3 to cd3713f Compare June 9, 2026 16:47

github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 9, 2026

Wallgau requested a review from mpawlow June 11, 2026 17:49

Vchen7629 approved these changes Jun 11, 2026

View reviewed changes

mpawlow requested changes Jun 11, 2026

View reviewed changes

fix: drop raw exception from docling-preset 500 detail and add 400 re…

6b457b8

…gression test (langflow-ai#1586)

github-actions Bot added tests bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 12, 2026

mpawlow approved these changes Jun 12, 2026

View reviewed changes

Merge branch 'main' into fix/1586-docling-preset-status

c2b0984

github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jun 15, 2026

github-actions Bot added the bug 🔴 Something isn't working. label Jun 15, 2026

coderabbitai Bot reviewed Jun 15, 2026

View reviewed changes

hunterxtang mentioned this pull request Jun 16, 2026

fix: preserve HTTPException status in update_docling_preset #1906

Closed

hunterxtang closed this Jun 18, 2026

hunterxtang mentioned this pull request Jul 2, 2026

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

Merged

Uh oh!

Conversation

hunterxtang commented Jun 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

Vchen7629 left a comment

Choose a reason for hiding this comment

Uh oh!

mpawlow left a comment

Choose a reason for hiding this comment

Code Review 1

Uh oh!

mpawlow Jun 11, 2026

Choose a reason for hiding this comment

(1a) [Normal] Raw exception message leaked in 500 response detail string

Problem

Code References

Potential Solution

Uh oh!

mpawlow Jun 11, 2026

Choose a reason for hiding this comment

(1b) [Minor] No regression test added to guard the fixed behavior

Problem

Code References

Potential Solution

Uh oh!

hunterxtang commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpawlow commented Jun 12, 2026

Uh oh!

mpawlow commented Jun 12, 2026

Uh oh!

mpawlow commented Jun 12, 2026

Uh oh!

mpawlow left a comment

Choose a reason for hiding this comment

Code Review 2

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mpawlow commented Jun 16, 2026

Uh oh!

mpawlow commented Jun 16, 2026

Uh oh!

Wallgau commented Jun 16, 2026

Uh oh!

mpawlow commented Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hunterxtang commented Jun 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

(1a) [Normal] Raw exception message leaked in 500 response `detail` string

hunterxtang commented Jun 12, 2026 •

edited

Loading