From 9c20e66ff8e54cb8456b136c4d7a9751d0793624 Mon Sep 17 00:00:00 2001 From: Facundo Rodriguez Date: Mon, 9 Feb 2026 15:51:44 -0300 Subject: [PATCH] fix(auth): prevent ERR_SERVER_NOT_RUNNING on magic-link login; harden stopServer --- src/commands/auth/login.ts | 59 +++++++++++++++++++++++++++++--- test/commands/auth/login.test.ts | 42 +++++++++++++++++++++-- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index aa017be6..0ea96711 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -6,7 +6,7 @@ import { Command } from '@oclif/core'; import { ensureUserSetup } from '../../api/user-setup.client.ts'; import { persistTokenResponse } from '../../service/auth.svc.ts'; import { getClientId, getRealmUrl } from '../../service/auth-config.svc.ts'; -import { getErrorMessage } from '../../service/log.svc.ts'; +import { debugLogger, getErrorMessage } from '../../service/log.svc.ts'; import type { TokenResponse } from '../../types/auth.ts'; import { openInBrowser } from '../../utils/open-in-browser.ts'; @@ -14,6 +14,7 @@ export default class AuthLogin extends Command { static description = 'OAuth CLI login'; private server?: http.Server; + private stopServerPromise?: Promise; private readonly port = parseInt(process.env.OAUTH_CALLBACK_PORT || '4000', 10); private readonly redirectUri = process.env.OAUTH_CALLBACK_REDIRECT || `http://localhost:${this.port}/oauth2/callback`; private readonly realmUrl = getRealmUrl(); @@ -136,11 +137,59 @@ export default class AuthLogin extends Command { }); } - private async stopServer() { - if (this.server) { - await new Promise((resolve, reject) => this.server?.close((err) => (err ? reject(err) : resolve()))); - this.server = undefined; + private stopServer(): Promise { + if (this.stopServerPromise) { + return this.stopServerPromise; } + + const server = this.server; + this.server = undefined; + + if (!server) { + return Promise.resolve(); + } + + const stopPromise = new Promise((resolve) => { + const timeoutMs = 1000; + let settled = false; + let timeout: ReturnType | undefined; + + const complete = (err?: Error) => { + if (settled) { + return; + } + + settled = true; + if (timeout) { + clearTimeout(timeout); + timeout = undefined; + } + + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (err && code !== 'ERR_SERVER_NOT_RUNNING') { + this.warn('Failed to stop local OAuth callback server.'); + debugLogger('Failed to stop local OAuth callback server: %s', getErrorMessage(err)); + } + + resolve(); + }; + + timeout = setTimeout(() => { + debugLogger('Timed out while stopping local OAuth callback server after %dms', timeoutMs); + complete(); + }, timeoutMs); + + try { + server.close((err) => complete(err)); + } catch (err) { + complete(err as Error); + } + }).finally(() => { + this.stopServerPromise = undefined; + }); + + this.stopServerPromise = stopPromise; + return stopPromise; } private async exchangeCodeForToken(code: string, codeVerifier: string): Promise { diff --git a/test/commands/auth/login.test.ts b/test/commands/auth/login.test.ts index 03b3c31d..89c6af1e 100644 --- a/test/commands/auth/login.test.ts +++ b/test/commands/auth/login.test.ts @@ -20,8 +20,18 @@ interface ServerStub { const serverInstances: ServerStub[] = []; +class ServerNotRunningError extends Error implements NodeJS.ErrnoException { + code = 'ERR_SERVER_NOT_RUNNING'; + + constructor() { + super('Server is not running.'); + this.name = 'ServerNotRunningError'; + } +} + const createServerStub = (handler: ServerHandler): ServerStub => { let errorListener: ((error: Error) => void) | undefined; + let closed = false; const stub: ServerStub = { handler, listen: vi.fn((_port: number, cb?: () => void) => { @@ -32,7 +42,9 @@ const createServerStub = (handler: ServerHandler): ServerStub => { return stub; }), close: vi.fn((cb?: (err?: Error) => void) => { - cb?.(); + const closeError = closed ? new ServerNotRunningError() : undefined; + closed = true; + setImmediate(() => cb?.(closeError)); return stub; }), on: vi.fn((event: string, cb: (err: Error) => void) => { @@ -80,8 +92,8 @@ vi.mock('../../../src/utils/open-in-browser.ts', () => ({ openInBrowser: vi.fn(), })); -const questionMock = vi.fn<[string, (answer: string) => void], void>(); -const closeMock = vi.fn<[], void>(); +const questionMock = vi.fn<(question: string, callback: (answer: string) => void) => void>(); +const closeMock = vi.fn<() => void>(); vi.mock('node:readline', () => ({ __esModule: true, @@ -303,6 +315,30 @@ describe('AuthLogin', () => { warnSpy.mockRestore(); } }); + + it('deduplicates shutdown when callback success and server error race', async () => { + const command = createCommand(basePort + 8); + const state = 'expected-state'; + const pendingCode = ( + command as unknown as { startServerAndAwaitCode: (url: string, state: string) => Promise } + ).startServerAndAwaitCode(authUrl, state); + const server = getLatestServer(); + const warnSpy = vi + .spyOn(command as unknown as { warn: (...args: unknown[]) => unknown }, 'warn') + .mockImplementation(() => {}); + + try { + await flushAsync(); + sendCallbackThroughStub({ code: 'race-code', state }); + server.emitError(new Error('late listener error')); + + await expect(pendingCode).resolves.toBe('race-code'); + expect(server.close).toHaveBeenCalledTimes(1); + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining('Failed to stop local OAuth callback server')); + } finally { + warnSpy.mockRestore(); + } + }); }); describe('exchangeCodeForToken', () => {