fix: stop swallowing failed chat responses as empty success - #5
Open
yhurynovich wants to merge 6 commits into
Open
fix: stop swallowing failed chat responses as empty success#5yhurynovich wants to merge 6 commits into
yhurynovich wants to merge 6 commits into
Conversation
The generic error fallback in handleApiError() could produce an empty-string `error` field when neither response.error nor response.statusText were set (common with Node's undici fetch on plain non-2xx responses). Every `if (result.error)` check across routes.js treats an empty string as falsy, so real upstream failures (e.g. Qwen's "Model not found") were silently returned to clients as a 200 with empty content and zeroed usage instead of a proper error. - Guarantee handleApiError's fallback error is always a non-empty string, falling back to `HTTP <status>` when no better message is available - Bump error-body logging from debug to warn so Qwen's raw error response is visible in logs without changing LOG_LEVEL - Add DEBUG_SSE_CHUNKS-gated raw SSE chunk logging for future streaming diagnostics
Problem
When Qwen returns an HTTP 429 (rate limit), the account gets cooled down via markRateLimitedByToken(). This cooldown was always defaulting to 24 hours, even when Qwen's own response says the wait is much shorter (e.g. 33 minutes).
Root cause
Two call sites in src/api/chat.js parse the 429 error body to get the wait time:
js
const rateInfo = JSON.parse(response.errorBody);
const parsedHours = Number(rateInfo.num);
Qwen's actual error body nests the wait duration under data.num, not top-level num:
json
{
"success": false,
"data": {
"code": "RateLimited",
"template": "You have reached the daily usage limit. Please wait {{num}} minutes before trying again.",
"num": 33
}
}
Since rateInfo.num is always undefined, Number.isFinite() fails and the code silently falls back to the hardcoded 24-hour default — ~40x longer than the 33 minutes Qwen actually requested. On top of the wrong path, the value is also documented by Qwen's own template string as minutes, while the code treats it as hours.
Fix
Read the wait value from data.num instead of top-level num.
Convert minutes to hours (/ 60) before passing to markRateLimitedByToken(), since that function multiplies by 3600 * 1000.
Falls back to 24h if the field is missing/non-numeric, preserving prior safe behavior.
Applied identically at both call sites (v2 sendMessage flow and the legacy v1 flow).
Impact
Accounts hitting Qwen's rate limit now cool down for the actual duration Qwen specifies instead of a flat 24h — significant for single-account deployments, where an inflated cooldown means total downtime.
Testing
node --check src/api/chat.js passes
Verified against a captured 429 response body (data.num: 33, template confirms "minutes")
Confirmed no other call sites read .num (grep -n "\.num\b" src/api/*.js)
Note
Only one 429 sample was available to confirm the "minutes" unit via data.template. If a RateLimited response is ever seen with a different template/unit, this assumption should be revisited.
Behavior:
- When a user sends {"message": "/new"} to /api/chat or /api/chat/completions, the browser session is restarted
- The message is NOT forwarded to the Qwen model
- Returns success/failure JSON response
…Docker Changes made: 1. src/config.js (line 58): Increased default PROTOCOL_TIMEOUT from 300,000ms (5 min) to 600,000ms (10 min) 2. src/browser/browser.js (lines 15-36): Disabled the problematic sourceurl stealth evasion which intercepts CDP commands and was causing the timeout 3. .env.example (line 88): Updated the documented default to 600000 Why this fixes it: - The sourceurl evasion wraps client.send() to strip Puppeteer's sourceURL from injected scripts. In Docker with Chromium, this interception adds overhead that can exceed the protocol timeout during heavy page operations. - Disabling it removes this overhead while keeping all other stealth evasions (webdriver, navigator plugins, user-agent, etc.) that prevent bot detection. - The increased timeout provides a safety margin for slower Docker environments. To apply: Rebuild your Docker image and restart the container. The fix will take effect automatically since the default is now 10 minutes and the problematic evasion is disabled. You can also override via environment variable if needed: PROTOCOL_TIMEOUT=900000 # 15 minutes
Critical Fixes Verified Working: 1. Blocking prompts fixed - NON_INTERACTIVE=1 or no TTY fails fast 2. Race conditions fixed - Mutex serializes browser lifecycle 3. Health checks added - Auto-recovers from browser crashes 4. Retry logic - 3 attempts with exponential backoff on launch 5. Session cleanup - Keeps only 10 most recent sessions 6. Security flags removed - No --disable-web-security, cert ignores 7. Path traversal blocked - Sanitized env dir names 8. No credential leaks - Verified logs only show counts/names
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: stop swallowing failed chat responses as empty success
The generic error fallback in handleApiError() could produce an
empty-string
errorfield when neither response.error norresponse.statusText were set (common with Node's undici fetch on
plain non-2xx responses). Every
if (result.error)check acrossroutes.js treats an empty string as falsy, so real upstream
failures (e.g. Qwen's "Model not found") were silently returned
to clients as a 200 with empty content and zeroed usage instead
of a proper error.
string, falling back to
HTTP <status>when no better messageis available
response is visible in logs without changing LOG_LEVEL
streaming diagnostics