fix(translate): enforce 1500-char text limit + add request timeout#220
Merged
Conversation
Two related stability issues hit during real-world use:
1. **Hung requests** — without an explicit timeout the upstream HTTP
call could dangle indefinitely on a stuck connection. Browser
extensions calling /translate would sit on a spinner forever with
no error to surface to the user (reported in the field).
2. **No-feedback on oversized input** — the oneshot endpoint caps the
total text length at 1500 characters (matches the extension's own
\`G.notLoggedIn = 1500\` constant). We were forwarding the request
anyway and letting DeepL 400 it, which a) wasted an upstream round
trip and b) the caller had no way to distinguish from other 400s.
Changes:
- Pre-validate \`text\` length in characters (utf8.RuneCountInString,
not byte length — verified the cap is rune-based: 1500 Chinese
characters / 4500 bytes is accepted, 1501 is rejected). Return
HTTP 413 Payload Too Large with a clear message naming both the
observed length and the limit.
- Set a 20s timeout on the oneshot HTTP client (req.SetTimeout).
On timeout return HTTP 504 Gateway Timeout — distinguishes a slow
DeepL from other 503 failure modes (DNS, TLS, etc.). The check
catches both context.DeadlineExceeded and url.Error{Timeout()=true}.
- Set a separate 5s timeout on the cookie-jar warmup GET to
www.deepl.com. Warmup is best-effort; we'd rather a slow warmup
(cookies still seed eventually next time) than block the very first
translation behind a hung GET.
Behaviour verified against the live oneshot endpoint:
- 1500 ASCII chars → 200
- 1501 ASCII chars → 413 (upstream not contacted)
- 1500 Chinese chars (4500 bytes) → 200
- 1501 Chinese chars → 413
- Pathological "your"*1500 → 504 at 20s (was hanging without timeout)
- Realistic 245-char Chinese → 200 in ~13s
Each TranslateByDeepLX call was building a brand-new req.Client via newOneshotClient(), which meant a fresh TLS handshake + HTTP/2 SETTINGS negotiation per request — ~200-400ms of pure overhead on top of DeepL's own ~1.5s processing latency. Share one client per proxy URL (sync.Map) so subsequent requests reuse the kept-alive HTTP/2 connection in the underlying http.Transport's pool. Also flip the cookie-jar warmup from synchronous-on-first-call to fire-and-forget at first client creation. Same sync.Once semantics (runs exactly once per process), but in a background goroutine so the first translate request runs in parallel with the TLS handshake to www.deepl.com rather than serially behind it. Measured against the live oneshot endpoint (Tokyo → Frankfurt): before, 5 sequential requests: 3.19s, 2.05s, 2.07s, 2.89s, 2.22s after, 5 sequential requests: 2.20s, 1.27s, 1.26s, 1.42s, 1.34s └─ first └────────── warm path ─────┘ The warm-path 1.3s is also faster than a bare \`curl\` to oneshot (~1.9s, every call doing its own TLS handshake) — proof the connection-pool reuse is now actually paying off.
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
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.
Summary
Two related stability fixes after we hit them in real-world use of the v1.2.x oneshot path.
1. Hung requests on slow upstream
Without an explicit HTTP timeout, a stuck DeepL connection would dangle indefinitely. Browser extensions calling `/translate` would sit on a spinner forever with no error to surface to the user.
Fix: `req.SetTimeout(20s)` on the oneshot client. On timeout return HTTP `504 Gateway Timeout` (distinguishable from generic 503 — "DeepL was slow" vs "connection refused / DNS failure / etc."). The check catches both `context.DeadlineExceeded` and `url.Error{Timeout()=true}` so it works whether the deadline came from `httpClient.Timeout` or a per-request `SetContext`.
The cookie-jar warmup GET to `www.deepl.com` gets its own shorter 5s timeout — warmup is best-effort, no point making the very first translate call wait 20s for a hung warmup.
2. No-feedback on oversized input
The oneshot endpoint hard-caps the total `text` array length at 1500 characters (verified upstream — matches the extension's own `G.notLoggedIn = 1500` constant in background.js). Past it, DeepL returns:
```json
{
"status": 400,
"errors": {"text": ["text exceeds maximum length"]},
"text_character_limit": 1500
}
```
We were happily forwarding any payload upstream, wasting a round trip, and the caller had to parse a generic 503 to figure out what went wrong.
Fix: Pre-validate `text` length in characters (`utf8.RuneCountInString`, not byte length — verified the upstream cap is rune-based: 1500 Chinese characters / 4500 bytes is accepted, 1501 is rejected). Return HTTP 413 Payload Too Large with a clear message naming the observed length and the limit:
```json
{
"code": 413,
"message": "text exceeds maximum length: 1501 characters (anonymous oneshot limit is 1500)"
}
```
Verification
Against the live oneshot endpoint:
Status code summary