[danscMax]

Problem

The dynamic OAuth GET /api/oauth/[provider]/[action] handler swallowed every thrown error into a generic { error: "Internal server error" } 500. So a device-code upstream failure — qwen (qwen.ai), codebuddy-cn (copilot.tencent.com) — surfaced in the companion extension as an indistinguishable 'ошибка сервера OmniRoute', hiding why it failed (geo-block, upstream outage, bad client). requestDeviceCode already throws a descriptive message (Device code request failed: …, CodeBuddy state request failed (403)) — it was just discarded.

Fix

Route the caught error through sanitizeErrorMessage() (already imported in the file; hard rule #12) and return it, falling back to the generic string only when the sanitizer yields nothing.

Tests

tests/unit/oauth-device-code-error-transparency.test.ts — regression guard (fails on the pre-fix base, passes after). Source-level: the route needs the full Next request/auth/upstream graph, so behavioural validation (a real device-code failure) belongs on a build/VPS per hard rule #18.

[gemini-code-assist]

Code Review

This pull request updates the OAuth GET handler to surface sanitized upstream error messages instead of returning a generic 500 Internal Server Error, improving transparency for issues like geo-blocks or upstream outages. A regression test was added to verify this behavior. The reviewer suggested replacing the current file-reading regression test with a more robust behavioral unit test using mocking, which is a recommended improvement for maintainability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

While adding a regression guard is a good step, testing the source code directly with readFileSync and regex is brittle. This approach couples the test to implementation details (like variable names), and a simple refactor could break it unnecessarily.

A more robust approach is to write a behavioral unit test that verifies the catch block's logic directly. This can be done by importing the GET handler, triggering its error path, and asserting on the NextResponse using mocks.

Here's a suggestion for a more conventional and maintainable unit test using node:test's mocking features:

import { test, mock, spyOn } from "node:test";
import assert from "node:assert/strict";
import { GET } from "@/app/api/oauth/[provider]/[action]/route";
import * as errorUtils from "@omniroute/open-sse/utils/error";
import { NextResponse } from "next/server";

// Suppress console.error during this test
spyOn(console, "error").mock.mockImplementation(() => {});

// Mock NextResponse.json to inspect its arguments
const jsonMock = mock.fn((body, init) => ({ body, init, headers: new Headers() }));
mock.method(NextResponse, "json", jsonMock);

test("OAuth GET catch block should return a sanitized error message", async () => {
  const testError = new Error("Upstream failure: Geoblocked");
  const sanitizedMessage = "Upstream failure: Geoblocked";
  const sanitizeSpy = spyOn(errorUtils, "sanitizeErrorMessage").mock.mockImplementation(
    () => sanitizedMessage
  );

  const mockRequest = new Request("http://localhost/api/oauth/qwen/device-code");
  // Trigger the catch block by rejecting the params promise
  const mockContext = { params: Promise.reject(testError) };

  await GET(mockRequest, mockContext as any);

  assert.strictEqual(jsonMock.mock.callCount(), 1, "NextResponse.json should be called once");
  const [body, init] = jsonMock.mock.calls[0].arguments;

  assert.deepStrictEqual(body, { error: sanitizedMessage });
  assert.deepStrictEqual(init, { status: 500 });

  assert.strictEqual(sanitizeSpy.mock.callCount(), 1, "sanitizeErrorMessage should be called once");
  assert.strictEqual(
    sanitizeSpy.mock.calls[0].arguments[0],
    testError.message,
    "sanitizeErrorMessage should be called with the error message"
  );
});

[diegosouzapw]

Thanks — this is a clean, narrowly-scoped fix. Confirmed no base drift (real diff = exactly the 2 files in the PR body) and the regression test passes on your branch while failing against the pre-fix release tip. sanitizeErrorMessage() is reused correctly per our error-sanitization convention, and the messages you're surfacing (qwen.ai/copilot.tencent.com upstream response bodies) don't carry our own secrets, so no new leak surface. Only optional note (echoing the automated review): the regression test currently pins the source text of the catch block via regex rather than exercising GET() end-to-end with a mocked failure — fine as-is given the route's dependency graph, but a behavioral test would be a nice follow-up if you have bandwidth. Planning to merge this alongside #7517 (the actual endpoint-URL fix) since they're complementary with zero file overlap.

[diegosouzapw]

Thanks @danscMax! Merged into release/v3.8.49 after combined merge-train validation (all gates green). For the image PRs we extracted the Google Imagen entry into its own registry module inside your branch (file-size cap on imageRegistry.ts) — you stay author throughout.