From 5c1e636dbe23b5eba1a3c39801c1e4f90c3b4e87 Mon Sep 17 00:00:00 2001
From: harsh <99705329+Ojhaharsh@users.noreply.github.com>
Date: Fri, 24 Apr 2026 12:13:13 +0530
Subject: [PATCH] fix: Strengthen error handling in qwenOAuth2.ts to prevent
 unhandled 'error' event (#3481)

* fix: strengthen error handling in launchBrowser to prevent unhandled events

* fix: strengthen error handling with ChildProcess type and debugLogger

* fix: use type-only import for ChildProcess
---
 packages/core/src/qwen/qwenOAuth2.ts | 46 ++++++++++++++++------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/packages/core/src/qwen/qwenOAuth2.ts b/packages/core/src/qwen/qwenOAuth2.ts
index 946835706..68512c09b 100644
--- a/packages/core/src/qwen/qwenOAuth2.ts
+++ b/packages/core/src/qwen/qwenOAuth2.ts
@@ -8,7 +8,7 @@ import crypto from 'crypto';
 import path from 'node:path';
 import { promises as fs } from 'node:fs';
 import * as os from 'os';
-
+import type { ChildProcess } from 'node:child_process';
 import open from 'open';
 import { EventEmitter } from 'events';
 import type { Config } from '../config/config.js';
@@ -489,10 +489,10 @@ export enum QwenOAuth2Event {
 export type AuthResult =
   | { success: true }
   | {
-      success: false;
-      reason: 'timeout' | 'cancelled' | 'error' | 'rate_limit';
-      message?: string; // Detailed error message for better error reporting
-    };
+    success: false;
+    reason: 'timeout' | 'cancelled' | 'error' | 'rate_limit';
+    message?: string; // Detailed error message for better error reporting
+  };
 
 /**
  * Global event emitter instance for QwenOAuth2 authentication events
@@ -717,26 +717,34 @@ async function authWithQwenDeviceFlow(
 
   // Helper to handle browser launch with error handling
   const launchBrowser = async (url: string): Promise<void> => {
-    try {
-      const childProcess = await open(url);
+    let childProcess: ChildProcess | undefined;
 
-      // IMPORTANT: Attach an error handler to the returned child process.
-      // Without this, if `open` fails to spawn a process (e.g., `xdg-open` is not found
-      // in a minimal Docker container), it will emit an unhandled 'error' event,
-      // causing the entire Node.js process to crash.
-      if (childProcess) {
-        childProcess.on('error', (err) => {
-          debugLogger.debug('Browser launch failed:', err.message || err);
+    try {
+      // Call open and get the process
+      childProcess = await open(url);
+
+      // CRITICAL FIX: Attach error listener IMMEDIATELY if a process object exists.
+      // We do this outside the 'if' check scope to ensure it's bound as soon as possible.
+      if (childProcess && typeof childProcess.on === 'function') {
+        childProcess.on('error', (err: Error) => {
+          debugLogger.warn(`Browser launch process error: ${err.message}`);
+          debugLogger.info(`Please open this URL manually: ${url}`);
         });
+
+        // Optional: Also listen for 'close' or 'exit' if needed for cleanup, 
+        // but 'error' is the main crasher.
+      } else {
+        // Fallback: If open() didn't return a valid process object, log a warning
+        debugLogger.debug('open() did not return a valid child process object.');
       }
+
     } catch (err) {
-      debugLogger.debug(
-        'Failed to open browser:',
-        err instanceof Error ? err.message : 'Unknown error',
-      );
+      // Handle synchronous errors or promise rejections from open()
+      const errorMessage = err instanceof Error ? err.message : String(err);
+      debugLogger.warn(`Failed to open browser automatically: ${errorMessage}`);
+      debugLogger.info(`Please open this URL manually: ${url}`);
     }
   };
-
   try {
     // Generate PKCE code verifier and challenge
     const { code_verifier, code_challenge } = generatePKCEPair();