fix(auth): prevent ERR_SERVER_NOT_RUNNING on magic-link login; harden stopServer (#474)

Author: facundo-herodevs

src/commands/auth/login.ts

@@ -6,14 +6,15 @@ 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';
 
 export default class AuthLogin extends Command {
   static description = 'OAuth CLI login';
 
   private server?: http.Server;
+  private stopServerPromise?: Promise<void>;
   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<void>((resolve, reject) => this.server?.close((err) => (err ? reject(err) : resolve())));
-      this.server = undefined;
+  private stopServer(): Promise<void> {
+    if (this.stopServerPromise) {
+      return this.stopServerPromise;
     }
+
+    const server = this.server;
+    this.server = undefined;
+
+    if (!server) {
+      return Promise.resolve();
+    }
+
+    const stopPromise = new Promise<void>((resolve) => {
+      const timeoutMs = 1000;
+      let settled = false;
+      let timeout: ReturnType<typeof setTimeout> | 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<TokenResponse> {

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<string> }
+      ).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', () => {