Skip to content

Commit 085fccb

Browse files
missBergclaudenacx
authored
fix: prevent nil translator panic when backend schema is unsupported (#1952)
**Description** When the ExtProc cannot create a translator for an unsupported backend schema, `GetTranslator` returns an error. However, `rp.upstreamFilter = u` was assigned *before* translator creation. When the response arrived, the router's nil check (`if r.upstreamFilter != nil`) passed because `u` existed, but `u.translator` was nil — causing a panic in `ProcessResponseHeaders`. This moves the `rp.upstreamFilter = u` assignment to after successful translator creation. If `GetTranslator` fails, `upstreamFilter` stays nil and the existing guard at `processor_impl.go:164` routes the response to `passThroughProcessor` instead of panicking. **Related Issues/PRs (if applicable)** Fixes #1941 **Special notes for reviewers (if applicable)** `rp.upstreamFilterCount++` intentionally remains before the translator check — it tracks the number of `SetBackend` calls (including failed ones) and is used by `onRetry()` to detect retry scenarios. This is unchanged from the previous behavior. Signed-off-by: Erica Hughberg <erica.sundberg.90@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Ignasi Barrera <ignasi@tetrate.io>
1 parent 331665a commit 085fccb

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

internal/extproc/processor_impl.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,13 +590,13 @@ func (u *upstreamProcessor[ReqT, RespT, RespChunkT, EndpointSpecT]) SetBackend(c
590590
if u.modelNameOverride != "" {
591591
u.requestHeaders[internalapi.ModelNameHeaderKeyDefault] = u.modelNameOverride
592592
}
593-
rp.upstreamFilter = u
594-
u.parent = rp
593+
u.parent = rp // Set parent before GetTranslator so it can access rp.eh
595594

596595
u.translator, err = u.parent.eh.GetTranslator(b.Schema, u.modelNameOverride)
597596
if err != nil {
598597
return fmt.Errorf("failed to create translator for backend %s: %w", b.Name, err)
599598
}
599+
rp.upstreamFilter = u // Only assign after translator is confirmed valid
600600

601601
switch redactor := u.translator.(type) {
602602
case translator.ResponseRedactor:

internal/extproc/processor_impl_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,43 @@ func Test_chatCompletionProcessorUpstreamFilter_SetBackend(t *testing.T) {
433433
require.Zero(t, mm.inputTokenCount)
434434
mm.RequireSelectedBackend(t, "some-backend")
435435
require.Equal(t, r, p.parent)
436+
// Verify upstreamFilter is NOT set when translator creation fails.
437+
// This prevents a nil-translator panic when the router processes the response
438+
// (the nil check on upstreamFilter at ProcessResponseHeaders/ProcessResponseBody
439+
// must fall through to passThroughProcessor).
440+
require.Nil(t, r.upstreamFilter, "upstreamFilter must remain nil when SetBackend fails")
441+
}
442+
443+
// Test_chatCompletionProcessorUpstreamFilter_SetBackend_unsupportedSchema_noResponsePanic
444+
// verifies that when SetBackend fails due to an unsupported schema, subsequent
445+
// response processing does not panic. Before the fix for #1941, upstreamFilter
446+
// was assigned before the translator was created, so the router's nil check on
447+
// upstreamFilter would pass but the nil translator would cause a panic.
448+
func Test_chatCompletionProcessorUpstreamFilter_SetBackend_unsupportedSchema_noResponsePanic(t *testing.T) {
449+
headers := map[string]string{":path": "/foo"}
450+
mm := &mockMetrics{}
451+
p := &chatCompletionProcessorUpstreamFilter{
452+
requestHeaders: headers,
453+
metrics: mm,
454+
}
455+
r := &chatCompletionProcessorRouterFilter{}
456+
457+
err := p.SetBackend(t.Context(), &filterapi.Backend{
458+
Name: "bad-backend",
459+
Schema: filterapi.VersionedAPISchema{Name: "unsupported-schema", Version: "v1"},
460+
}, nil, r)
461+
require.Error(t, err)
462+
require.Nil(t, r.upstreamFilter, "upstreamFilter must remain nil on translator creation failure")
463+
464+
// Simulate response arriving after the failed SetBackend.
465+
// This must NOT panic; it should fall through to passThroughProcessor.
466+
resp, err := r.ProcessResponseHeaders(t.Context(), nil)
467+
require.NoError(t, err)
468+
require.NotNil(t, resp)
469+
470+
resp, err = r.ProcessResponseBody(t.Context(), &extprocv3.HttpBody{Body: []byte("error"), EndOfStream: true})
471+
require.NoError(t, err)
472+
require.NotNil(t, resp)
436473
}
437474

438475
func Test_chatCompletionProcessorUpstreamFilter_ProcessRequestHeaders(t *testing.T) {

0 commit comments

Comments
 (0)