mirror of
https://github.com/thedotmack/claude-mem.git
synced 2026-09-20 04:23:02 +08:00
fix(worker): classify Anthropic invalid_request_error as unrecoverable when .status is dropped by SDK (closes #2656) (#2673)
* fix(worker): classify Anthropic 400 invalid_request_error as unrecoverable when .status is dropped by SDK wrapping (#2656) The Anthropic Agent SDK sometimes surfaces 400 invalid_request errors as wrapped exceptions where the `.status` field is lost in the wrapping — only the canonical message text (or a structured `error.type === 'invalid_request_error'`) survives. Before this fix the existing `errAny.status === 400` branch in `classifyClaudeError` was the only guard; without `.status` the error fell through to the default `transient` branch and the worker retried the same permanent configuration error indefinitely while `/health` kept reporting `ok` — exactly the symptom reported in #2656 for "The provided model identifier is invalid". Add a conservative message-anchored fallback that catches three canonical shapes: - `error.type === 'invalid_request_error'` (Anthropic API body shape) - the canonical phrase "the provided model identifier is invalid" - any message that carries `invalid_request_error` as a token Anchored on full phrases / `\b` word boundaries so unrelated messages containing the word "invalid" still classify as `transient` (verified by a regression test). The pre-existing `.status === 400` branch continues to handle statused 400s first, so the effort-hint logging path for #2357 is untouched. Adds 5 regression tests; the existing 7 still pass. * fix(worker): guard status-less invalid_request_error branch against statused 5xx (#2656) Addresses the Greptile review on #2673. The status-less invalid_request_error fallback ran before the 5xx branch with no status guard, so an error carrying `status: 5xx` plus an `invalid_request_error` body/message was classified `unrecoverable` and halted retries for what is a transient server error. Add `typeof errAny.status !== 'number'` so the branch only handles genuinely status-less errors (its stated intent); statused 4xx/5xx stay on their own branches. Also trims the 7-line block comment down to match the one-line comment style of the sibling branches in classifyClaudeError. Adds a regression test: status 503 + invalid_request_error body -> transient. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: youcefzemmar <dzbuildoff@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,6 +136,19 @@ export function classifyClaudeError(err: unknown): ClassifiedProviderError {
|
||||
);
|
||||
}
|
||||
|
||||
// Status-less Anthropic 400s — SDK wrapping can drop `.status`, leaving only
|
||||
// the message or an `invalid_request_error` body; classify those as
|
||||
// unrecoverable so the worker stops retrying a permanent config error (#2656).
|
||||
// The status guard keeps statused 4xx/5xx on their own branches.
|
||||
if (
|
||||
typeof errAny.status !== 'number' &&
|
||||
(errAny.error?.type === 'invalid_request_error' ||
|
||||
/\bthe provided model identifier is invalid\b/i.test(message) ||
|
||||
/\binvalid_request_error\b/i.test(message))
|
||||
) {
|
||||
return new ClassifiedProviderError(message, { kind: 'unrecoverable', cause: err });
|
||||
}
|
||||
|
||||
// Server errors → transient.
|
||||
if (typeof errAny.status === 'number' && errAny.status >= 500 && errAny.status < 600) {
|
||||
return new ClassifiedProviderError(message, { kind: 'transient', cause: err });
|
||||
|
||||
@@ -120,3 +120,74 @@ describe('classifyClaudeError — sibling status codes (regression sanity)', ()
|
||||
expect(classified.kind).toBe('transient');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Regression coverage for #2656: when the Anthropic Agent SDK wraps a 400
|
||||
* `invalid_request_error` (e.g. "The provided model identifier is invalid")
|
||||
* the `.status` field can be lost in the wrapping. Without a message-based
|
||||
* fallback the error fell through to the default `transient` branch and the
|
||||
* worker retried indefinitely while `/health` kept reporting `ok`.
|
||||
*/
|
||||
describe('classifyClaudeError — model identifier rejections without .status (#2656)', () => {
|
||||
let warnSpy: ReturnType<typeof spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
__resetEffortHintLatchForTesting();
|
||||
warnSpy = spyOn(logger, 'warn').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
warnSpy.mockRestore();
|
||||
__resetEffortHintLatchForTesting();
|
||||
});
|
||||
|
||||
it('classifies "The provided model identifier is invalid" as unrecoverable even without a status field', () => {
|
||||
const sdkErr = new Error('The provided model identifier is invalid');
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
expect(classified.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('classifies wrapped errors exposing error.type=invalid_request_error as unrecoverable', () => {
|
||||
const sdkErr = Object.assign(
|
||||
new Error('Anthropic SDK error'),
|
||||
{ error: { type: 'invalid_request_error' } },
|
||||
);
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
expect(classified.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('classifies errors carrying the "invalid_request_error" string in the message as unrecoverable', () => {
|
||||
const sdkErr = new Error('Request failed: invalid_request_error from upstream');
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
expect(classified.kind).toBe('unrecoverable');
|
||||
});
|
||||
|
||||
it('does not match unrelated messages containing the word "invalid"', () => {
|
||||
const sdkErr = new Error('Some unrelated invalid input from a tool');
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
// Must NOT be unrecoverable just because the word "invalid" appears —
|
||||
// matching is anchored on the canonical Anthropic phrases only.
|
||||
expect(classified.kind).toBe('transient');
|
||||
});
|
||||
|
||||
it('still routes statused 400s through the existing branch (does not fall through)', () => {
|
||||
const sdkErr = Object.assign(
|
||||
new Error('The provided model identifier is invalid'),
|
||||
{ status: 400 },
|
||||
);
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
expect(classified.kind).toBe('unrecoverable');
|
||||
// The pre-existing status=400 branch handles this case before the new
|
||||
// fallback runs; no effort-hint should fire (no effort marker present).
|
||||
expect(warnSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps a statused 5xx carrying invalid_request_error transient (status guard)', () => {
|
||||
const sdkErr = Object.assign(
|
||||
new Error('gateway error: invalid_request_error from upstream'),
|
||||
{ status: 503, error: { type: 'invalid_request_error' } },
|
||||
);
|
||||
const classified = classifyClaudeError(sdkErr);
|
||||
expect(classified.kind).toBe('transient');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user