fix(relay): 请求参数校验错误返回 HTTP 400 by ax2 · Pull Request #6774 · QuantumNous/new-api

ax2 · 2026-08-11T09:11:20Z

⚠️ 提交说明 / PR Notice

Important

请提供人工撰写的简洁摘要，避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

现象

Chat Completions、Embeddings、Responses 和 Claude Messages 在本地请求校验阶段发现缺少必填参数时，错误信息是正确的，但 HTTP 状态码为 500。客户端、监控和网关会因此把输入错误误判为服务端故障。

根因

四种协议最终都经过 controller.Relay 调用 helper.GetAndValidateRequest。校验失败后，请求体过大已有单独的 413 分支；其余错误虽然使用了 invalid_request 错误码，但通过 types.NewError 构造时沿用了默认的 500 状态码。

修复

在 Relay 的公共校验错误边界显式设置 HTTP 400，并增加 skipRetry。这样无需分别修改各协议 DTO 或校验函数，所有本地 invalid_request 保持一致。
保留原有错误响应结构：OpenAI 兼容入口仍返回 error.code = "invalid_request"，Claude Messages 仍返回 Claude 错误包络。
保留请求体过大时的 413 分支，不改变渠道选择、上游转发和计费流程。
新增控制器级表驱动测试，覆盖以下四种缺少必填参数的请求：
- /v1/chat/completions 缺少 messages
- /v1/embeddings 缺少 input
- /v1/responses 缺少 input
- /v1/messages 缺少 messages

影响范围

变更只作用于 GetAndValidateRequest 返回错误、且错误不属于请求体过大的路径。合法请求以及进入渠道选择后的错误处理不受影响。

🚀 变更类型 / Type of change

🐛 Bug 修复 (Bug fix) - 请关联对应 Issue，避免将设计取舍、理解偏差或预期不一致直接归类为 bug
✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
⚡ 性能优化 / 重构 (Refactor)
📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

Closes Bug: 请求参数校验错误在多种 Relay 协议下返回 HTTP 500 #6773

✅ 提交前检查项 / Checklist

人工确认: 我已亲自整理并撰写此描述，没有直接粘贴未经处理的 AI 输出。
非重复提交: 我已搜索现有的 Issues 与 PRs，确认不是重复提交。
Bug fix 说明: 若此 PR 标记为 Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
变更理解: 我已理解这些更改的工作原理及可能影响。
范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
本地验证: 已在本地运行并通过测试或手动验证，维护者可以据此复核结果。
安全合规: 代码中无敏感凭据，且符合项目代码规范。

📸 运行证明 / Proof of Work

修复前运行新增回归测试，四个子用例均稳定得到 actual: 500；修复后：

$ go test ./controller -run '^TestRelayReturnsBadRequestForInvalidClientParameters$' -count=1
ok  github.com/QuantumNous/new-api/controller

$ GOWORK=off go vet ./...
通过

$ GOWORK=off go build ./...
通过

$ make test
根模块与 relaykit 模块全部通过

$ cd relaykit && GOWORK=off go vet ./... && GOWORK=off go build ./...
通过

$ cd web && bun run typecheck
通过

web/bun test 在未修改的前端代码上仍有主线既有失败：3 个 Auto group UI 断言失败，以及 6 个 Bun 对嵌套 describe 的 ERR_NOT_IMPLEMENTED。本 PR 不修改前端；相关测试迁移已有 #6569，当前上游 PR #6772 的 Frontend typecheck and test 也显示相同门禁失败。

Summary by CodeRabbit

Bug Fixes
- Invalid request payloads now consistently return HTTP 400 responses while preserving existing error details and retry behavior.
- Oversized request bodies continue to return HTTP 413 responses.
Tests
- Added validation coverage for missing required parameters across chat completions, embeddings, responses, and messaging requests.

coderabbitai · 2026-08-11T09:11:50Z

Walkthrough

The relay now returns HTTP 400 for invalid request parameters while preserving HTTP 413 for oversized bodies. Table-driven tests cover missing required parameters across four OpenAI and Claude relay formats.

Changes

Relay validation

Layer / File(s)	Summary
Validation status and regression coverage `controller/relay.go`, `controller/relay_validation_test.go`	The relay assigns HTTP 400 to validation errors other than oversized request bodies. Tests cover missing required parameters for chat completions, embeddings, responses, and Claude messages, including format-specific error payloads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: calcium-ion

Poem

A rabbit checks each request with care,
Bad fields now find a 400 there.
Four formats hop through tests in line,
While oversized bodies keep 413 as sign.
Validation stays neat and fine.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes fix HTTP 400 handling for all four required Relay protocols and add matching regression tests while preserving 413 and non-retry behavior.
Out of Scope Changes check	✅ Passed	All changes support the linked issue and PR objectives; no unrelated code changes are identified.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	标题明确概括了 Relay 请求参数校验错误改为返回 HTTP 400 的主要变更，与 PR 内容一致。

✨ 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.}

…am PR QuantumNous#6774) controller/relay.go: wrap the invalid_request error with an explicit 400 status (ErrOptionWithStatusCode + skipRetry) so empty-messages / missing-field requests return 400 instead of 500. Matches upstream QuantumNous/new-api PR QuantumNous#6774. Build-env fixes (CN network): Dockerfile sets GOPROXY=goproxy.cn and switches apt to the tuna debian mirror (deb.debian.org/proxy.golang.org hang from here); .dockerignore excludes runtime dirs (data-local/logs-local/backups-local) that were bloating the build context to 20GB.

* feat: glm chanel /v1/responses (QuantumNous#7050) * feat(ollama): passthrough Claude Messages and OpenAI Responses (QuantumNous#7051) * docs: update PR template and remove PR Check workflow (QuantumNous#7053) * docs: update PR template and remove PR Check workflow * docs: add hidden agent issue and PR templates * fix(web): restore admin unbinding for built-in providers (QuantumNous#6987) * fix(web): align admin binding types Refs QuantumNous#6985 * test(web): restore animation mock * fix(billing): 修复时间规则恒真表达式导致倍率全天生效 (QuantumNous#6934) Co-authored-by: seefs001 <i@seefs.me> * fix(docker): add relaykit go.mod to dev build context (QuantumNous#7072) * feat(task): replace built-in task adaptors with a sandboxed JS plugin system (QuantumNous#7076) * fix(relay): 请求参数校验错误返回 HTTP 400 (QuantumNous#6774) * fix(relay): return 400 for invalid request parameters * fix(web): recheck setup status after page reload (QuantumNous#6968) * feat(auth): encrypt password login transport Closes QuantumNous#6743 --------- Co-authored-by: Seefs <40468931+seefs001@users.noreply.github.com> Co-authored-by: zcxads666 <128150298+zcxads666@users.noreply.github.com> Co-authored-by: seefs001 <i@seefs.me> Co-authored-by: Uladzislau <53997152+VladKabiak@users.noreply.github.com> Co-authored-by: Calcium-Ion <i@caion.me> Co-authored-by: Alex Xiang <ax2@zicode.com>

* fix(web): restore admin unbinding for built-in providers (QuantumNous#6987) * fix(web): align admin binding types Refs QuantumNous#6985 * test(web): restore animation mock * fix(billing): 修复时间规则恒真表达式导致倍率全天生效 (QuantumNous#6934) Co-authored-by: seefs001 <i@seefs.me> * fix(docker): add relaykit go.mod to dev build context (QuantumNous#7072) * feat(task): replace built-in task adaptors with a sandboxed JS plugin system (QuantumNous#7076) * fix(relay): 请求参数校验错误返回 HTTP 400 (QuantumNous#6774) * fix(relay): return 400 for invalid request parameters * fix(web): recheck setup status after page reload (QuantumNous#6968) * feat(auth): encrypt password login transport Closes QuantumNous#6743 * feat(chat): add AQBot preset (QuantumNous#7079) * feat(auth): make password encryption opt-in QuantumNous#6743 * feat(task): resolve channel-mapped aliases and case variants for plugin models Channel model_mapping keys exposed in a channel's model list now act as first-class aliases for task-plugin models across the whole line: - Derived alias view (model/task_model_alias.go): built from enabled channels' model_mapping, chain-following with cycle detection, declared names always win, cross-plugin conflicts dropped. Rebuilt on channel cache refresh, registry generation change, and a 60s TTL. - Request path: PinTaskPluginEndpoint resolves declared-name case folds and mapping aliases before endpoint lookup (never rewriting the body until the endpoint is claimed), pins with MappedModel, and the decode contract accepts alias echoes without loosening model ownership for normal pins. Legacy /v1/tasks submit folds case variants the same way. Fixes aliases on POST /v1/responses silently falling through to the main relay against task channels. - Mapping order: ModelMappedHelper now runs before the plugin submit hook builds and caches the upstream body, so channel model_mapping actually reaches the upstream request. Plugins receive the mapped name as ctx.upstreamModel in both decode and submit contexts. - Billing: identity stays the origin name; when the alias has no tiered expression, the selected channel's mapping tail expression applies. Pricing page and billing-expr smoke tests resolve aliases to the owning plugin's usage schema. - Case folding: ASCII-only fold with exact-match priority; same-plugin and cross-plugin fold collisions rejected at registration. - Plugins: model-keyed rate tables, req_key derivation, and combo validation in doubao/kling/jimeng/hailuo/vidu/sunoapi now key on ctx.upstreamModel || ctx.model; render/echo paths keep ctx.model. * fix(model): disable PostgreSQL prepared statements for pooler compatibility GORM v1.25.2 closes cached prepared statements asynchronously on any SQL error and immediately re-Parses the same deterministic name (pgx's stmt_<sha256>) on the same client connection. Transaction-pooling proxies (PgBouncer >=1.21 with max_prepared_statements, Neon, Supabase) respond with FATAL "prepared statement name is already in use" (SQLSTATE 08P01) and drop the connection. PreferSimpleProtocol only disables pgx's implicit prepare and never covered GORM's explicit PrepareStmt cache. - PostgreSQL now runs with PrepareStmt disabled entirely; named prepared statements are fundamentally session state and cannot be made safe under transaction pooling. Parse/plan cost is noise for this workload. - Upgrade gorm to v1.25.12 so MySQL/SQLite statement caches (still enabled) no longer churn close/re-prepare on ordinary SQL errors; v1.25.9+ restricts eviction to driver.ErrBadConn. Deliberately not v1.26+, whose LRU eviction has an open use-after-close race (#7831). - sanitizeDBError now attaches a remediation hint on 08P01/42P05 so affected deployments can self-diagnose from the log line. * fix(ali): honor image response format (QuantumNous#5513) (QuantumNous#7048) * feat(web): factory task plugins update only with the system Marketplace install/upgrade on a factory-served plugin actually created a permanent override shadowing every future built-in release. The card now shows an informational "Updates with the system" badge instead of the action, while keeping the built-in vs marketplace version line and the upgradable state badge visible. Deliberate overrides are untouched: upload and marketplace actions on overridden or third-party plugins behave as before, and the plugins table now hints when an override lags behind the shipped built-in version so operators know deleting it restores the newer factory plugin. * fix(subscription): 无有效订阅时前端如实显示「仅用订阅」偏好 (QuantumNous#6222) (QuantumNous#7086) Co-authored-by: Claude <noreply@anthropic.com> * fix(sqlite): enable WAL + working busy timeout + _txlock=immediate to stop concurrent write lockouts (QuantumNous#7030) * fix(sqlite): enable WAL + working busy timeout + _txlock=immediate to stop concurrent write lockouts * fix(model): return string from JSON column Valuers for pg simple protocol With PrepareStmt disabled, PostgreSQL queries run over pgx's simple protocol, which encodes every []byte parameter as a bytea hex literal ('\x...'). driver.Valuer implementations returning []byte from json.Marshal therefore fail json-column writes with SQLSTATE 22P02 (reported on the channels UPDATE path via ChannelInfo). Reproduced against a live PostgreSQL 16: []byte Valuer into a json column fails under simple protocol, string succeeds; []byte into a text column silently stores the hex literal (no such path exists in the repo today — audited all Valuers, json.RawMessage fields, and raw SQL call sites). - ChannelInfo, Properties, TaskPrivateData, JSONValue Value() now return string; zero-value nil semantics unchanged. Task.Data (bare json.RawMessage) is unaffected — database/sql's default converter already passes it as expected. - Their Scan() counterparts now accept both []byte and string via a shared jsonScanBytes helper: SQLite returns string for these columns once Value() emits string, and the old []byte-only assertions silently zeroed the field (caught by the model test suite). - Add regression tests locking both contracts: json-column Valuers must return string (or nil for zero values), Scanners must accept []byte and string. Verified end-to-end against PostgreSQL 16 with the real model types: Channel create/update/read-back, Task json fields, PrefillGroup items. * fix(relay): bound the wait for upstream response headers (fixes unbounded heap growth → OOM) (QuantumNous#6949) * fix(relay): bound the wait for upstream response headers (fixes unbounded heap growth) The relay transport sets a dial timeout, a TLS handshake timeout and an expect-continue timeout, but nothing bounds how long it waits for the upstream *response headers* after the request has been written. An upstream that accepts the connection and then never answers -- without sending FIN/RST, which is what happens when a NAT/firewall silently drops the flow or the provider hangs -- parks the goroutine in net/http.(*persistConn).roundTrip forever. That goroutine keeps the whole request alive, which in practice means three copies of the request body stay reachable for the lifetime of the process: the raw bytes from io.ReadAll in CreateBodyStorageFromReader, the decoded messages held as json.RawMessage, and the re-marshalled upstream body from common.Marshal. BodyStorageCleanup cannot help here: it runs after c.Next() returns, and for these requests c.Next() never returns. Measured on v1.0.0-rc.23 in production (see QuantumNous#6947 for the full evidence): - 23 goroutines stuck in persistConn.roundTrip on a single 40h-old instance, blocked between 353 and 1894 minutes (5.9h to 31.5h) - 96.9% of the live heap, sampled after a forced GC, attributable to those three body copies (HeapAlloc 892 MiB surviving three GC cycles; HeapObjects dropping 30x while bytes dropped only 25%) - the live floor grows with uptime: 33.7 MiB at 0.1h, 89.2 at 13.8h, 510.0 at 40.1h, 955.2 at 146.8h, OOMKilled at 172.9h -- same image, same config, same load Doubling the memory limit and adding GOMEMLIMIT only moved the OOM from 132h to 172.9h. RELAY_TIMEOUT (http.Client.Timeout) cannot be used for this: it covers the whole response read and would cut legitimate long streaming calls, which is why it defaults to 0. ResponseHeaderTimeout only bounds the wait for the headers; streaming after they arrive is unaffected. The default is deliberately generous. Non-streaming upstreams usually send the response headers only once generation has finished, so the value has to leave room for a long completion. 1800s is 12x shorter than the shortest hang observed here while leaving several times the headroom a normal non-streaming request needs; 0 restores the previous unbounded behaviour. The assignment goes next to the other transport.* lines rather than inside the else branch: newRelayHTTPTransport() normally takes the http.DefaultTransport.Clone() path, and DefaultTransport does not set ResponseHeaderTimeout either. This repo already sets ResponseHeaderTimeout on its other outbound transports (controller/model_sync.go, controller/ratio_sync.go); the relay path appears to have been missed. Refs QuantumNous#6947. Likely also the root cause of QuantumNous#6731, which reported the same symptom (production OOM on /v1/responses after ~64h) but was closed for template reasons. * review: clamp overflowing timeout values and switch the test to testify Addresses the two CodeRabbit findings on this PR. Overflow (common/init.go:113): a RELAY_RESPONSE_HEADER_TIMEOUT beyond ~9.2e9 seconds overflows time.Duration and can wrap into a *tiny positive* timeout, which would cut every relay request instead of only the stuck ones. The value is now clamped before the conversion, with regression tests for both the negative and the overflowing input. I did not add fail-on-startup validation for negative values, for two reasons: the existing `if seconds > 0` guard already treats them as "disabled", and the neighbouring env-driven timeouts in this file are less strict still -- RelayIdleConnTimeout is converted with no guard at all. Failing startup on a bad value would be a behaviour change out of step with the rest of the file; happy to add it if you'd prefer that direction repo-wide. Test style: switched to testify (require.Equal / require.Zero / require.Positive), which is what every other test under service/ uses. go build, go vet and go test ./common/... ./service/... pass. (`go build ./...` fails on the `web/dist` embed both with and without this change -- the frontend bundle is not checked in.) * fix initialize database * fix(model): drop leftover prefill_groups unique constraints before AutoMigrate (QuantumNous#7100) * Revert "fix(model): drop leftover prefill_groups unique constraints before Au…" (QuantumNous#7101) This reverts commit 69a41ee. * fix(model): drop leftover prefill_groups unique constraints before AutoMigrate --------- Co-authored-by: zcxads666 <128150298+zcxads666@users.noreply.github.com> Co-authored-by: seefs001 <i@seefs.me> Co-authored-by: Uladzislau <53997152+VladKabiak@users.noreply.github.com> Co-authored-by: Calcium-Ion <i@caion.me> Co-authored-by: Alex Xiang <ax2@zicode.com> Co-authored-by: Seefs <40468931+seefs001@users.noreply.github.com> Co-authored-by: 憧憬Licoy <licoycn@gmail.com> Co-authored-by: PuppetKL <154485567+PuppetKL@users.noreply.github.com> Co-authored-by: ruiyunzhao <91191418+CR-Yun@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Xayinn <129403670+LinineTy@users.noreply.github.com> Co-authored-by: txgo <tianxi.liu@gmail.com>

* fix(relay): return 400 for invalid request parameters

* fix(relay): return 400 for invalid request parameters (cherry picked from commit 0f2a207)

fix(relay): return 400 for invalid request parameters

68d67b5

ax2 marked this pull request as ready for review August 11, 2026 09:15

Delete controller/relay_validation_test.go

ad05be3

Calcium-Ion merged commit 0f2a207 into QuantumNous:main Aug 29, 2026
1 check passed

chunfeng789 mentioned this pull request Aug 30, 2026

Merge upstream QuantumNous/new-api into main (sync through 2b6f1dfef) chunfeng789/new-api#9

Merged

5 tasks

drwoodck pushed a commit to drwoodck/new-api that referenced this pull request Sep 2, 2026

fix(relay): 请求参数校验错误返回 HTTP 400 (QuantumNous#6774)

ab7641d

* fix(relay): return 400 for invalid request parameters

yiranxiaohui pushed a commit to yiranxiaohui/new-api that referenced this pull request Sep 2, 2026

fix(relay): 请求参数校验错误返回 HTTP 400 (QuantumNous#6774)

211b2c6

* fix(relay): return 400 for invalid request parameters (cherry picked from commit 0f2a207)

Navigation Menu

Uh oh!

fix(relay): 请求参数校验错误返回 HTTP 400 - #6774

fix(relay): 请求参数校验错误返回 HTTP 400#6774
Calcium-Ion merged 2 commits into
QuantumNous:mainfrom
ax2:agent/fix-relay-validation-status

ax2 commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ax2 commented Aug 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ 提交说明 / PR Notice

📝 变更描述 / Description

现象

根因

修复

影响范围

🚀 变更类型 / Type of change

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

📸 运行证明 / Proof of Work

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ax2 commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading