Skip to content

hotfix(ui): reset networking.test.ts to upstream — companion to #62 - #63

Merged
songkuan-zheng merged 1 commit into
ship/v1.87.0from
fix/v1.87.0-reset-networking-test
Jun 4, 2026
Merged

hotfix(ui): reset networking.test.ts to upstream — companion to #62#63
songkuan-zheng merged 1 commit into
ship/v1.87.0from
fix/v1.87.0-reset-networking-test

Conversation

@songkuan-zheng

Copy link
Copy Markdown
Collaborator

Same root cause as #62. Before: 5/21 tests fail (handleErrorResponse missing). After: 18/18 pass. UI 401 tests deferred to same follow-up PR as 401 impl.

#62

Same root cause as #62: Wave 7's `git checkout --theirs` on
`networking.test.ts` preserved our UI 401 test additions but
referenced `handleErrorResponse` — a function that only existed
in our cherry-pick version of `networking.tsx`. After #62 reset
`networking.tsx` to upstream, the tests pointed at a now-nonexistent
symbol.

Before: 5 of 21 tests fail with `TypeError: handleErrorResponse is
not a function`.

After: 18 of 18 upstream tests pass.

Our UI 401 test logic is deferred to the same follow-up PR that
re-applies the 401-redirect logic onto upstream-clean networking.tsx.

Tier: B (internal infra hotfix; final piece of Wave 7's misresolved
--theirs).
@songkuan-zheng
songkuan-zheng merged commit 7d571ea into ship/v1.87.0 Jun 4, 2026
@songkuan-zheng
songkuan-zheng deleted the fix/v1.87.0-reset-networking-test branch June 4, 2026 12:23
songkuan-zheng added a commit that referenced this pull request Jun 4, 2026
Codifies the lessons from PRs #62, #63 — Wave 7's misuse of
`git checkout --theirs` on `networking.tsx` / `networking.test.ts`
silently dropped upstream's Memory CRUD additions and broke the Docker
build. Two follow-up PRs were needed to recover.

New `CLAUDE.md` section "Conflict resolution discipline" (placed right
after "Upstream sync cadence") with 5 rules:

1. Default to manual 3-way merge for any large file or upstream-active
   file (router.py, proxy_server.py, networking.tsx, auth/*, ...).
2. `--ours` is acceptable when our patch ships content via separate
   files (Wave 6c pattern).
3. `--theirs` is almost never correct; if tempted, instead take HEAD
   as the base and re-apply the cherry-pick's logic onto it.
4. After any conflict resolution, verify line-count sanity (`git diff
   --stat`), build/import smoke, and targeted tests — skipping any of
   these is what let PR #61 ship broken.
5. PR description must document the per-file strategy + verification
   evidence (line-count diff + build/test smoke outputs).

`.github/pull_request_template.md` gains a "Conflict resolutions"
section between "Tier classification" and "Relevant issues" so the
strategy + evidence land in every cherry-pick PR going forward.

Tier: B (internal infra / policy doc).
songkuan-zheng added a commit that referenced this pull request Jun 4, 2026
…#66)

`bash -n e2e/tools/run-all-cases` failed with "syntax error: unexpected
end of file" at line 611. Root cause: during one of the Wave 6
cherry-pick conflict resolutions on this file, the closing `fi` (for
the `if [ "$rc" -eq 0 ]` inside case_23) and the function-body `}` were
dropped — case_23's body ran off into the post-function preamble code.

Fix: re-insert the missing `fi` and `}` after the `fail` branch.

Verified:
  bash -n e2e/tools/run-all-cases  # syntax OK
  e2e/tools/run-all-cases --help   # works
  e2e/tools/run-all-cases --mock-only  # runs all 22 cases

This is the 3rd silent regression caused by conflict-resolution sloppiness
during the v1.87.0 bump (after PR #62 networking.tsx and #63
networking.test.ts). See CLAUDE.md → "Conflict resolution discipline".

Tier: B (internal infra hotfix).
songkuan-zheng added a commit that referenced this pull request Jun 5, 2026
…working.tsx (#68)

Wave 7 of the v1.87.0 bump originally added structured auth error
handling to ui/litellm-dashboard/src/components/networking.tsx via
three commits:

  - be8a895  fix(ui): route 401/403 by backend error.type, heuristic only as fallback (#30)
  - f73ae47  fix(ui): redirect to login on session-expired 401, not on all 401s
  - 0383afb  fix(ui): redirect to login on 401 token_not_found_in_db (#38)

PR #61 dropped them with a careless `git checkout --theirs` during the
v1.87.0 cherry-pick. PRs #62 and #63 reset networking.tsx +
networking.test.ts to upstream verbatim to unblock the Docker build.
This PR re-layers that work on top of the upstream-clean baseline.

The matching backend change (`_classify_auth_failure` in
litellm/proxy/auth/auth_exception_handler.py, Wave 6a) is already in
production. It emits `error.type` with one of:

  - auth_session_expired       → REDIRECT_LOGIN
  - auth_invalid_credentials   → REDIRECT_LOGIN
  - token_not_found_in_db      → REDIRECT_LOGIN (legacy specific type)
  - expired_key                → REDIRECT_LOGIN (legacy specific type)
  - auth_permission_denied     → TOAST (the original bug)
  - *_model_access_denied      → TOAST
  - team_member_permission_error → TOAST
  - budget_exceeded            → TOAST
  - auth_error                 → HEURISTIC (legacy marker fallback)

Three layers added to networking.tsx:

1. AUTH_ERROR_TYPE_TO_ACTION dispatch table + extractErrorType helper.
2. Status-aware handleErrorResponse(response, errorData) — preferred
   when caller has the Response object. Three-tier decision:
   structured type → status+cookie heuristic → delegate to legacy.
3. Legacy handleError(errorData) now also reads error.type FIRST.
   This was the live-e2e bug fix (commit 2 of #30): the ~30 existing
   fetch sites call handleError directly, not handleErrorResponse, so
   making the legacy entry point smart auto-propagates the D1 taxonomy
   contract without touching call sites.

Callsite migration (commit 3 of #38, Patch 1): 163 fetch sites in
networking.tsx were calling handleError(errorMessage) — pre-stringifying
the body before handleError could read error.type. Bulk-replaced with
handleError(errorData) so the structured-type path actually fires in
production. The MCP tool call path has a special errorData scope
(declared inside `try`), so hoist `let errorData: any = null` outside
the try and pass `errorData ?? responseText` to handleError.

Tests: 38 new test cases across three describe blocks:
  - handleErrorResponse - status-aware auth handling (5 tests)
  - handleErrorResponse - type-based auth routing (D1 contract) (14 tests)
  - handleError (legacy) - now also reads error.type (8 tests)
  - existing networking - expired session handling (3 tests, unchanged)
  Total: 45 tests pass (was 7).

Verification:
- npm run build → ✓ Compiled successfully in 26.2s, 37 static pages
- npx vitest run src/components/networking.test.ts → 45/45 pass

Tier: C (universal bug fix — auth error UX correctness)
Tried upstream first? No — this is companion code to our backend
_classify_auth_failure (Wave 6a) which is itself an internal Tier D
mechanism. The structured-type contract is the carry; upstream may
adopt a similar taxonomy independently.

Conflict resolutions: N/A (clean re-application on upstream-clean
baseline, no `git cherry-pick` was used — manual port of the
three source commits' intent onto upstream's current networking.tsx
structure).
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