fix: include JSON parse error details

[mturac]

Fixes #3468.

Summary

• Preserve the HTTP 400 response for default JSON parse failures.
• Include the parser/unmarshal detail in the integration error message instead of returning only Invalid JSON.
• Add GenAI and generic default-parser regression coverage for malformed or schema-incompatible request bodies.

Tests

• GOWORK=/tmp/bifrost-go-work.<tmp> go test ./transports/bifrost-http/integrations -run 'TestCreateHandler_(GenAIUnmarshalErrorIncludesSchemaDetail|DefaultJSONParserFailureClosesConnection|CustomParserFailureClosesConnection|SkipsRequestParserInLargePayloadMode|UsesRequestParserWhenNotInLargePayloadMode)' -count=1\n- git diff --check\n\nNote: this is a clean, rebased replacement for fix: return bad request for schema parse errors #3493, which had picked up unrelated commits during the earlier rebase.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 076918fd-c2a6-4a35-b903-2a9e069fd052

📥 Commits

Reviewing files that changed from the base of the PR and between 2d17a58 and 0de50da.

📒 Files selected for processing (3)

• transports/bifrost-http/integrations/router.go
• transports/bifrost-http/integrations/router_large_payload_test.go
• transports/bifrost-http/integrations/router_test.go

✅ Files skipped from review due to trivial changes (1)

• transports/bifrost-http/integrations/router_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• transports/bifrost-http/integrations/router_large_payload_test.go
• transports/bifrost-http/integrations/router.go

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Enhanced error messages for invalid JSON requests to include specific parsing error details, helping users identify and fix malformed payloads more easily.

Walkthrough

In router.go, the JSON parse error message sent to clients on HTTP 400 is changed from the static string "Invalid JSON" to "Invalid JSON: <underlying error>". Two test files are updated: one tightens existing assertions to match the new format, and one adds a new GenAI-specific test verifying schema-detail appears in the 400 response body.

Changes

Richer JSON parse error message

Layer / File(s)	Summary
Propagate underlying error into Invalid JSON message transports/bifrost-http/integrations/router.go	The sendError call on JSON parse failure now appends the underlying error to the message: "Invalid JSON: <err>" instead of the static "Invalid JSON". HTTP 400 status and connection-close behavior are unchanged.
Test coverage for enriched error messages transports/bifrost-http/integrations/router_test.go, transports/bifrost-http/integrations/router_large_payload_test.go	Existing default-parser failure test updated to assert both "Invalid JSON: invalid JSON request body" and "invalid char" substrings. New TestCreateHandler_GenAIUnmarshalErrorIncludesSchemaDetail test added: configures a GenAI route with a gemini error converter, triggers a schema-mismatch unmarshal failure, and asserts a 400 response body containing "Invalid JSON" and "contents" field detail.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hopped by with a JSON mistake,
The error once silent, now wide awake!
"Invalid JSON: <why>" now greets every call,
With "contents" and "invalid char" listed for all.
No more mystery 500s in the night —
The parse error shines with a clear, hoppy light! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: include JSON parse error details' accurately describes the main change: enhancing error messages to include underlying JSON parse details instead of generic 'Invalid JSON'.
Description check	✅ Passed	The PR description is mostly complete with summary, test instructions, and issue reference, though it lacks some optional template sections like 'Type of change' checkboxes and 'Affected areas' structure.
Linked Issues check	✅ Passed	The changes fully address #3468 by including JSON parse error details in responses (changing from generic 'Invalid JSON' to 'Invalid JSON: '), preserving HTTP 400 status, and adding comprehensive test coverage for both GenAI and default parser failure scenarios.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: they modify error message handling in router.go, update related tests, and add new regression tests for JSON parse failures in GenAI routes—no out-of-scope modifications detected.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Confidence Score: 5/5

The change is a one-line message enrichment in the default JSON error path; it cannot affect routing, provider dispatch, or streaming behavior.

The fix is narrowly scoped: only the user-visible error message string changes, the HTTP status code and connection-close behaviour remain identical. The existing error is still stored in the BifrostError.Error field unchanged, and the new message simply prepends "Invalid JSON: " to the already-wrapped sonic error string. No control-flow, auth, or data-handling paths are touched. Tests cover both the generic and GenAI unmarshal code paths.

No files require special attention.

Important Files Changed

Filename	Overview
transports/bifrost-http/integrations/router.go	Single-line fix: the default JSON parse error message now includes fmt.Sprintf("Invalid JSON: %v", err) instead of the static "Invalid JSON", surfacing the underlying sonic/unmarshal detail to callers.
transports/bifrost-http/integrations/router_test.go	Updated TestCreateHandler_DefaultJSONParserFailureClosesConnection assertions to match the richer error message format, adding a check that the sonic "invalid char" detail is present.
transports/bifrost-http/integrations/router_large_payload_test.go	New regression test TestCreateHandler_GenAIUnmarshalErrorIncludesSchemaDetail sends a type-mismatched Gemini request body and asserts the 400 response contains both the "Invalid JSON: invalid JSON request body" prefix and the field name ("contents") from the sonic error.

_{Reviews (4): Last reviewed commit: "[fix]: transports - include JSON parse e..." | Re-trigger Greptile}

The merge-base changed after approval.

[mturac]

Closing this to avoid splitting review on the same issue. #4907 is already open and includes the GenAI schema-mismatch error path for #3468, so I will leave that as the active review path.