Skip to content

test(integration): opt the config pass-through spend-log case into auth (rc/1.105.0 backport of #44265) - #44269

Merged
yuneng-berri merged 1 commit into
rc/1.105.0from
litellm_test_43ffc0
Oct 2, 2026
Merged

yuneng-berri merged 1 commit into
rc/1.105.0from
litellm_test_43ffc0

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Scheduled CircleCI integration-extensions has been red since 2026-10-01 06:09 UTC
  • The config pass-through spend-log test relied on a config wins side effect

How it solves it:

  • The test's config route now sets auth: true explicitly
  • No product code changes, so pass-through auth stays as before config wins

User Flow

Before: a config pass-through route without auth is treated the way it was before config wins, and the test expects otherwise

  1. The admin declares /audit-pt under general_settings.pass_through_endpoints with no auth key
  2. A client sends POST http://localhost:4000/audit-pt with a valid key and the upstream answers 403
  3. The client gets the upstream 403 body back, and no failure row shows up in http://localhost:4000/ui/?page=logs

After: the same route with auth: true gets the failure row the test was written for

  1. The admin declares /audit-pt with auth: true
  2. The client sends the same POST http://localhost:4000/audit-pt and gets the upstream 403 body back
  3. http://localhost:4000/ui/?page=logs shows the failed request with the upstream error message

Relevant issues

Follow-up to #43962 and #42695. Backport of #44265 to rc/1.105.0

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally
  • My PR passes all required CI/CD checks
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

The one test, run twice on each side against a local integration stack (Postgres 16, Redis 7, the test's own 2-worker proxy): pytest --timeout=90 -p no:randomly tests/integration/observability/test_passthrough_upstream_error_visibility.py::test_config_pass_through_route_logs_body_and_strips_query

Before (485ad76)

  1. Run 1: Failed: Timeout (>90.0s) from pytest-timeout while polling for the failure row in _spend_error_information
  2. Run 2: same timeout at the same frame (92.4s)

After (1d9cd9b)

  1. Run 1: passed in 40.2s
  2. Run 2: passed in 44.2s
  3. LiteLLM_SpendLogs held exactly 2 failure rows afterwards, one per After run and none from the Before runs

Type

✅ Test

Caveats (if any)

Low

  • Failures on config pass-through routes without auth still write no spend-log row, which matches behavior before config wins

The failure spend-log row from #42695 is written for pass-through routes that
run as LLM API routes, which a config route only does with auth: true. The
test omitted auth and passed only while config wins (#41779) registered config
entries through the typed model, where auth defaults to true. #43962 restored
the pre-config-wins registration, so the route lost that status and the row
was never written. Set auth: true on the route so the test covers the logging
it was written for without depending on that side effect

(cherry picked from commit 1d9cd9b)
@yuneng-berri
yuneng-berri merged commit 80cbbbd into rc/1.105.0 Oct 2, 2026
8 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_test_43ffc0 branch October 2, 2026 23:11
@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low risk] Test file adds a configuration option to an integration test.

This isolated test change appears safe to merge

What we checked:

  • Authentication errors cannot pass unnoticed: The test checks the entire upstream response body, then checks its message in both the warning and spend row. A proxy authentication error would fail these checks

Summary

This test-only change adds explicit auth: True to the configured pass-through route so the test exercises authenticated failure logging

  • Keeps checks for the upstream response body, query stripping, and the failed spend-log row
  • No product code changes or actionable findings
  • yuneng-berri explicitly acknowledged that routes without auth still produce no failure spend-log row, matching the intended earlier behavior
  • Tests were not run during this review

Reviews (1) · Last reviewed commit: "test(integration): opt the config pass-t..."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant