diff --git a/src/auth/refresh.ts b/src/auth/refresh.ts index 2ab8daa..c0b97c7 100644 --- a/src/auth/refresh.ts +++ b/src/auth/refresh.ts @@ -1,28 +1,45 @@ -import type { OAuthTokens, CredentialFile } from './types'; -import { saveCredentials } from './credentials'; -import { CLIError } from '../errors/base'; -import { ExitCode } from '../errors/codes'; +import type { OAuthTokens, CredentialFile } from "./types"; +import { saveCredentials } from "./credentials"; +import { CLIError } from "../errors/base"; +import { ExitCode } from "../errors/codes"; // OAuth config — endpoints TBD pending MiniMax OAuth documentation -const TOKEN_URL = 'https://api.minimax.io/v1/oauth/token'; +const TOKEN_URL = "https://api.minimax.io/v1/oauth/token"; export async function refreshAccessToken( refreshToken: string, ): Promise { - const res = await fetch(TOKEN_URL, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams({ - grant_type: 'refresh_token', - refresh_token: refreshToken, - }), - }); + let res: Response; + try { + res = await fetch(TOKEN_URL, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + grant_type: "refresh_token", + refresh_token: refreshToken, + }), + signal: AbortSignal.timeout(10_000), + }); + } catch (err) { + const isTimeout = + err instanceof Error && + (err.name === "AbortError" || + err.name === "TimeoutError" || + err.message.includes("timed out")); + throw new CLIError( + isTimeout + ? "Token refresh timed out — auth server did not respond within 10 s." + : `Token refresh failed: ${err instanceof Error ? err.message : String(err)}`, + ExitCode.AUTH, + "Check your network connection.\nRe-authenticate: mmx auth login", + ); + } if (!res.ok) { throw new CLIError( - 'OAuth session expired and could not be refreshed.', + "OAuth session expired and could not be refreshed.", ExitCode.AUTH, - 'Re-authenticate: mmx auth login', + "Re-authenticate: mmx auth login", ); } @@ -45,7 +62,7 @@ export async function ensureFreshToken(creds: CredentialFile): Promise { access_token: tokens.access_token, refresh_token: tokens.refresh_token, expires_at: new Date(Date.now() + tokens.expires_in * 1000).toISOString(), - token_type: 'Bearer', + token_type: "Bearer", account: creds.account, }; diff --git a/src/config/detect-region.ts b/src/config/detect-region.ts index 4f33a21..22423c0 100644 --- a/src/config/detect-region.ts +++ b/src/config/detect-region.ts @@ -1,37 +1,60 @@ -import { REGIONS, type Region } from './schema'; -import { readConfigFile, writeConfigFile } from './loader'; +import { REGIONS, type Region } from "./schema"; +import { readConfigFile, writeConfigFile } from "./loader"; -const QUOTA_PATH = '/v1/api/openplatform/coding_plan/remains'; +const QUOTA_PATH = "/v1/api/openplatform/coding_plan/remains"; function quotaUrl(region: Region): string { return REGIONS[region] + QUOTA_PATH; } -async function probeRegion(region: Region, apiKey: string, timeoutMs: number): Promise { - try { - const res = await fetch(quotaUrl(region), { - headers: { 'Authorization': `Bearer ${apiKey}`, 'Content-Type': 'application/json' }, - signal: AbortSignal.timeout(timeoutMs), - }); - if (!res.ok) return false; - const data = await res.json() as { base_resp?: { status_code?: number } }; - return data.base_resp?.status_code === 0; - } catch { - return false; +async function probeRegion( + region: Region, + apiKey: string, + timeoutMs: number, +): Promise { + // MiniMax endpoints accept either Bearer or x-api-key auth — try both. + // Some API key types only work with one style; trying both prevents false + // negatives that would cause the wrong region to be selected, leading to + // every subsequent request timing out or returning 401. + const authHeaders: Record[] = [ + { Authorization: `Bearer ${apiKey}` }, + { "x-api-key": apiKey }, + ]; + + for (const authHeader of authHeaders) { + try { + const res = await fetch(quotaUrl(region), { + headers: { ...authHeader, "Content-Type": "application/json" }, + signal: AbortSignal.timeout(timeoutMs), + }); + if (!res.ok) continue; + const data = (await res.json()) as { + base_resp?: { status_code?: number }; + }; + if (data.base_resp?.status_code === 0) return true; + } catch { + // Try next auth style before giving up on this region + } } + return false; } export async function detectRegion(apiKey: string): Promise { - process.stderr.write('Detecting region...'); + process.stderr.write("Detecting region..."); const regions = Object.keys(REGIONS) as Region[]; const results = await Promise.all( - regions.map(async (r) => ({ region: r, ok: await probeRegion(r, apiKey, 5000) })), + regions.map(async (r) => ({ + region: r, + ok: await probeRegion(r, apiKey, 5000), + })), ); const match = results.find((r) => r.ok); if (!match) { - process.stderr.write(' failed\n'); - process.stderr.write('Warning: API key failed validation against all regions. Falling back to global.\n'); - return 'global'; + process.stderr.write(" failed\n"); + process.stderr.write( + "Warning: API key failed validation against all regions. Falling back to global.\n", + ); + return "global"; } const detected: Region = match.region; process.stderr.write(` ${detected}\n`); diff --git a/src/errors/handler.ts b/src/errors/handler.ts index ef1939a..acf978a 100644 --- a/src/errors/handler.ts +++ b/src/errors/handler.ts @@ -1,17 +1,17 @@ -import { CLIError } from './base'; -import { ExitCode } from './codes'; -import { detectOutputFormat } from '../output/formatter'; +import { CLIError } from "./base"; +import { ExitCode } from "./codes"; +import { detectOutputFormat } from "../output/formatter"; export function handleError(err: unknown): never { if (err instanceof CLIError) { const format = detectOutputFormat(process.env.MINIMAX_OUTPUT); - if (format === 'json') { - process.stderr.write(JSON.stringify(err.toJSON(), null, 2) + '\n'); + if (format === "json") { + process.stderr.write(JSON.stringify(err.toJSON(), null, 2) + "\n"); } else { process.stderr.write(`\nError: ${err.message}\n`); if (err.hint) { - process.stderr.write(`\n ${err.hint.split('\n').join('\n ')}\n`); + process.stderr.write(`\n ${err.hint.split("\n").join("\n ")}\n`); } process.stderr.write(` (exit code ${err.exitCode})\n`); } @@ -19,21 +19,28 @@ export function handleError(err: unknown): never { } if (err instanceof Error) { - if (err.name === 'AbortError' || err.message.includes('timed out')) { + if ( + err.name === "AbortError" || + err.name === "TimeoutError" || + err.message.includes("timed out") + ) { const timeout = new CLIError( - 'Request timed out.', + "Request timed out.", ExitCode.TIMEOUT, - 'Try increasing --timeout or retry later.', + "Try increasing --timeout (e.g. --timeout 60).\n" + + "If this happens on every request with a valid API key, you may be hitting the wrong region.\n" + + "Run: mmx auth status — to check your credentials and region.\n" + + "Run: mmx config set region global (or cn) — to override the region.", ); return handleError(timeout); } // Detect TypeError from fetch with invalid URL (e.g., malformed MINIMAX_BASE_URL) - if (err instanceof TypeError && err.message === 'fetch failed') { + if (err instanceof TypeError && err.message === "fetch failed") { const networkErr = new CLIError( - 'Network request failed.', + "Network request failed.", ExitCode.NETWORK, - 'Check your network connection and proxy settings. Also verify MINIMAX_BASE_URL is a valid URL.', + "Check your network connection and proxy settings. Also verify MINIMAX_BASE_URL is a valid URL.", ); return handleError(networkErr); } @@ -41,27 +48,28 @@ export function handleError(err: unknown): never { // Detect network-level errors (proxy, connection refused, DNS, etc.) const msg = err.message.toLowerCase(); const isNetworkError = - msg.includes('failed to fetch') || - msg.includes('connection refused') || - msg.includes('econnrefused') || - msg.includes('connection reset') || - msg.includes('econnreset') || - msg.includes('network error') || - msg.includes('enotfound') || - msg.includes('getaddrinfo') || - msg.includes('proxy') || - msg.includes('socket') || - msg.includes('etimedout') || - msg.includes('timeout') || - msg.includes('eai_AGAIN'); + msg.includes("failed to fetch") || + msg.includes("connection refused") || + msg.includes("econnrefused") || + msg.includes("connection reset") || + msg.includes("econnreset") || + msg.includes("network error") || + msg.includes("enotfound") || + msg.includes("getaddrinfo") || + msg.includes("proxy") || + msg.includes("socket") || + msg.includes("etimedout") || + msg.includes("timeout") || + msg.includes("eai_AGAIN"); if (isNetworkError) { - let hint = 'Check your network connection and proxy settings.'; - if (msg.includes('proxy')) { - hint = 'Proxy error — check HTTP_PROXY / HTTPS_PROXY environment variables and proxy authentication.'; + let hint = "Check your network connection and proxy settings."; + if (msg.includes("proxy")) { + hint = + "Proxy error — check HTTP_PROXY / HTTPS_PROXY environment variables and proxy authentication."; } const networkErr = new CLIError( - 'Network request failed.', + "Network request failed.", ExitCode.NETWORK, hint, ); @@ -71,36 +79,37 @@ export function handleError(err: unknown): never { // Detect filesystem-level errors (ENOENT, EACCES, ENOSPC, etc.) const ecode = (err as NodeJS.ErrnoException).code; if ( - ecode === 'ENOENT' || - ecode === 'EACCES' || - ecode === 'ENOSPC' || - ecode === 'ENOTDIR' || - ecode === 'EISDIR' || - ecode === 'EPERM' || - ecode === 'EBUSY' + ecode === "ENOENT" || + ecode === "EACCES" || + ecode === "ENOSPC" || + ecode === "ENOTDIR" || + ecode === "EISDIR" || + ecode === "EPERM" || + ecode === "EBUSY" ) { - let hint = 'Check the file path and permissions.'; - if (ecode === 'ENOENT') hint = 'File or directory not found.'; - if (ecode === 'EACCES' || ecode === 'EPERM') hint = 'Permission denied — check file or directory permissions.'; - if (ecode === 'ENOSPC') hint = 'Disk full — free up space and try again.'; + let hint = "Check the file path and permissions."; + if (ecode === "ENOENT") hint = "File or directory not found."; + if (ecode === "EACCES" || ecode === "EPERM") + hint = "Permission denied — check file or directory permissions."; + if (ecode === "ENOSPC") hint = "Disk full — free up space and try again."; const fsErr = new CLIError( `File system error: ${err.message}`, ExitCode.GENERAL, hint, ); return handleError(fsErr); - } else if (typeof ecode === 'string' && ecode.startsWith('E')) { + } else if (typeof ecode === "string" && ecode.startsWith("E")) { // All other Node.js filesystem error codes (EMFILE, EEXIST, EROFS, etc.) const fsErr = new CLIError( `File system error: ${err.message}`, ExitCode.GENERAL, - 'Check the file path and permissions.', + "Check the file path and permissions.", ); return handleError(fsErr); } process.stderr.write(`\nError: ${err.message}\n`); - if (process.env.MINIMAX_VERBOSE === '1') { + if (process.env.MINIMAX_VERBOSE === "1") { process.stderr.write(`${err.stack}\n`); } } else { diff --git a/test/auth/timeout-fix.test.ts b/test/auth/timeout-fix.test.ts new file mode 100644 index 0000000..407ca6f --- /dev/null +++ b/test/auth/timeout-fix.test.ts @@ -0,0 +1,229 @@ +/** + * Tests for auth timeout bug fixes: + * + * Bug 1 (detect-region.ts): Region probe only sent Bearer auth — keys that + * only work with x-api-key would fail all probes, fall back to 'global', + * and every subsequent request would time out or 401. + * + * Bug 2 (refresh.ts): Token refresh had no timeout — a slow/unreachable auth + * server caused the CLI to hang indefinitely. + * + * Bug 3 (handler.ts): Timeout errors showed a generic "try --timeout" hint + * with no guidance about wrong-region or auth issues. + */ + +import { describe, it, expect, afterEach } from 'bun:test'; +import { createMockServer, jsonResponse, type MockServer } from '../helpers/mock-server'; +import { CLIError } from '../../src/errors/base'; +import { ExitCode } from '../../src/errors/codes'; + +// --------------------------------------------------------------------------- +// Bug 1 — detect-region probes both Bearer and x-api-key auth styles +// --------------------------------------------------------------------------- + +describe('detect-region: probeRegion auth style fallback', () => { + let server: MockServer; + + afterEach(() => server?.close()); + + it('succeeds when endpoint only accepts Bearer token', async () => { + server = createMockServer({ + routes: { + '/v1/api/openplatform/coding_plan/remains': (req) => { + if (req.headers.get('Authorization') === 'Bearer bearer-only-key') { + return jsonResponse({ base_resp: { status_code: 0 } }); + } + return jsonResponse({ error: 'unauthorized' }, 401); + }, + }, + }); + + // Patch REGIONS to point at our mock server + const { REGIONS } = await import('../../src/config/schema'); + const origGlobal = REGIONS.global; + (REGIONS as Record).global = server.url; + + try { + const { detectRegion } = await import('../../src/config/detect-region'); + const region = await detectRegion('bearer-only-key'); + expect(region).toBe('global'); + } finally { + (REGIONS as Record).global = origGlobal; + } + }); + + it('succeeds when endpoint only accepts x-api-key header', async () => { + server = createMockServer({ + routes: { + '/v1/api/openplatform/coding_plan/remains': (req) => { + if (req.headers.get('x-api-key') === 'xapikey-only-key') { + return jsonResponse({ base_resp: { status_code: 0 } }); + } + return jsonResponse({ error: 'unauthorized' }, 401); + }, + }, + }); + + const { REGIONS } = await import('../../src/config/schema'); + const origGlobal = REGIONS.global; + (REGIONS as Record).global = server.url; + + try { + const { detectRegion } = await import('../../src/config/detect-region'); + const region = await detectRegion('xapikey-only-key'); + expect(region).toBe('global'); + } finally { + (REGIONS as Record).global = origGlobal; + } + }); + + it('falls back to global when key is invalid for all auth styles and regions', async () => { + server = createMockServer({ + routes: { + '/v1/api/openplatform/coding_plan/remains': () => + jsonResponse({ error: 'unauthorized' }, 401), + }, + }); + + const { REGIONS } = await import('../../src/config/schema'); + const origGlobal = REGIONS.global; + const origCn = REGIONS.cn; + (REGIONS as Record).global = server.url; + (REGIONS as Record).cn = server.url; + + try { + const { detectRegion } = await import('../../src/config/detect-region'); + const region = await detectRegion('bad-key'); + expect(region).toBe('global'); // graceful fallback + } finally { + (REGIONS as Record).global = origGlobal; + (REGIONS as Record).cn = origCn; + } + }); +}); + +// --------------------------------------------------------------------------- +// Bug 2 — token refresh has a 10-second timeout and clear error messages +// --------------------------------------------------------------------------- + +describe('refreshAccessToken: timeout and error handling', () => { + let server: MockServer; + + afterEach(() => server?.close()); + + it('throws a CLIError with AUTH exit code when refresh endpoint returns non-ok', async () => { + server = createMockServer({ + routes: { + '/v1/oauth/token': () => jsonResponse({ error: 'invalid_grant' }, 400), + }, + }); + + // Temporarily redirect TOKEN_URL to our mock + const mod = await import('../../src/auth/refresh'); + + // We test the real function against a mock server via a wrapper + // that overrides the fetch to hit our local server instead. + const origFetch = globalThis.fetch; + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString(); + if (url.includes('oauth/token')) { + return origFetch(`${server.url}/v1/oauth/token`, init); + } + return origFetch(input, init); + }; + + try { + await expect(mod.refreshAccessToken('expired-refresh-token')).rejects.toMatchObject({ + exitCode: ExitCode.AUTH, + }); + } finally { + globalThis.fetch = origFetch; + } + }); + + it('returns a fresh token when refresh succeeds', async () => { + server = createMockServer({ + routes: { + '/v1/oauth/token': () => + jsonResponse({ + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + expires_in: 3600, + token_type: 'Bearer', + }), + }, + }); + + const mod = await import('../../src/auth/refresh'); + const origFetch = globalThis.fetch; + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString(); + if (url.includes('oauth/token')) { + return origFetch(`${server.url}/v1/oauth/token`, init); + } + return origFetch(input, init); + }; + + try { + const tokens = await mod.refreshAccessToken('valid-refresh-token'); + expect(tokens.access_token).toBe('new-access-token'); + expect(tokens.expires_in).toBe(3600); + } finally { + globalThis.fetch = origFetch; + } + }); + + it('ensureFreshToken returns cached token when not near expiry', async () => { + const mod = await import('../../src/auth/refresh'); + const token = await mod.ensureFreshToken({ + access_token: 'still-valid', + refresh_token: 'refresh', + expires_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), // 1h from now + token_type: 'Bearer', + }); + expect(token).toBe('still-valid'); + }); +}); + +// --------------------------------------------------------------------------- +// Bug 3 — timeout error message includes auth / region diagnostic hints +// --------------------------------------------------------------------------- + +describe('handleError: timeout message includes region/auth hint', () => { + it('AbortError message contains region override hint', () => { + const { handleError } = require('../../src/errors/handler'); + + const abortErr = new DOMException('The operation was aborted.', 'AbortError'); + + let captured = ''; + const origWrite = process.stderr.write.bind(process.stderr); + (process.stderr as NodeJS.WriteStream).write = (chunk: unknown) => { + captured += String(chunk); + return true; + }; + + try { + handleError(abortErr); + } catch { + // process.exit throws in test env — that's expected + } finally { + (process.stderr as NodeJS.WriteStream).write = origWrite; + } + + expect(captured).toContain('mmx auth status'); + expect(captured).toContain('region'); + }); + + it('CLIError with TIMEOUT exit code shows correct hint', () => { + const err = new CLIError('Request timed out.', ExitCode.TIMEOUT, + 'Try increasing --timeout (e.g. --timeout 60).\n' + + 'If this happens on every request with a valid API key, you may be hitting the wrong region.\n' + + 'Run: mmx auth status — to check your credentials and region.\n' + + 'Run: mmx config set region global (or cn) — to override the region.', + ); + + expect(err.exitCode).toBe(ExitCode.TIMEOUT); + expect(err.hint).toContain('mmx auth status'); + expect(err.hint).toContain('mmx config set region'); + }); +});