fix: surface Python errors during agentcore dev (#359)

notgitika · gitikavj · web-flow · commit c7eead8c03c2 · 2026-02-23T14:48:00.000-05:00
* fix: surface Python errors during agentcore dev

* fix: address PR review comments

---------

Co-authored-by: gitikavj &lt;gitikavj@amazon.com&gt;
diff --git a/src/cli/operations/dev/__tests__/dev-server.test.ts b/src/cli/operations/dev/__tests__/dev-server.test.ts
@@ -218,4 +218,105 @@ describe('DevServer', () => {
       expect(onExit).toHaveBeenCalledWith(0);
     });
   });
+
+  describe('Python traceback detection', () => {
+    const TRACEBACK = [
+      'Traceback (most recent call last):',
+      '  File "/app/.venv/lib/python3.12/site-packages/uvicorn/server.py", line 86, in _serve',
+      '    config.load()',
+      '  File "/app/myagent/main.py", line 1, in <module>',
+      '    import nonexistent_package',
+      "ModuleNotFoundError: No module named 'nonexistent_package'",
+    ].join('\n');
+
+    it('emits only user-code frames and exception line for tracebacks', async () => {
+      await server.start();
+
+      mockChild.stderr.emit('data', Buffer.from(TRACEBACK));
+
+      const errorCalls = (onLog as ReturnType<typeof vi.fn>).mock.calls.filter((c: unknown[]) => c[0] === 'error');
+      const errorMessages = errorCalls.map((c: unknown[]) => c[1]);
+
+      // Should include user frame + code + exception, but NOT site-packages frame
+      expect(errorMessages).toContain('  File "/app/myagent/main.py", line 1, in <module>');
+      expect(errorMessages).toContain('    import nonexistent_package');
+      expect(errorMessages).toContain("ModuleNotFoundError: No module named 'nonexistent_package'");
+      // Should NOT include internal frames
+      expect(errorMessages).not.toContain(
+        '  File "/app/.venv/lib/python3.12/site-packages/uvicorn/server.py", line 86, in _serve'
+      );
+    });
+
+    it('does not emit traceback lines as info', async () => {
+      await server.start();
+
+      mockChild.stderr.emit('data', Buffer.from(TRACEBACK));
+
+      const infoCalls = (onLog as ReturnType<typeof vi.fn>).mock.calls.filter((c: unknown[]) => c[0] === 'info');
+      // None of the traceback lines should leak as info
+      for (const [, msg] of infoCalls) {
+        expect(msg).not.toContain('Traceback');
+        expect(msg).not.toContain('nonexistent_package');
+      }
+    });
+
+    it('resumes normal classification after traceback ends', async () => {
+      await server.start();
+
+      mockChild.stderr.emit('data', Buffer.from(TRACEBACK + '\nINFO: some normal log'));
+
+      expect(onLog).toHaveBeenCalledWith('info', 'INFO: some normal log');
+    });
+  });
+
+  describe('stderr crash buffer', () => {
+    it('emits buffered stderr as errors on non-zero exit', async () => {
+      await server.start();
+
+      // Emit some non-traceback stderr lines
+      mockChild.stderr.emit('data', Buffer.from('some debug output'));
+      mockChild.stderr.emit('data', Buffer.from('another line'));
+
+      mockChild.emit('exit', 1);
+
+      const errorCalls = (onLog as ReturnType<typeof vi.fn>).mock.calls.filter((c: unknown[]) => c[0] === 'error');
+      const errorMessages = errorCalls.map((c: unknown[]) => c[1]);
+
+      expect(errorMessages).toContain('some debug output');
+      expect(errorMessages).toContain('another line');
+      expect(onExit).toHaveBeenCalledWith(1);
+    });
+
+    it('does not emit stderr buffer on clean exit (code 0)', async () => {
+      await server.start();
+
+      mockChild.stderr.emit('data', Buffer.from('some debug output'));
+      mockChild.emit('exit', 0);
+
+      const errorCalls = (onLog as ReturnType<typeof vi.fn>).mock.calls.filter((c: unknown[]) => c[0] === 'error');
+      expect(errorCalls).toHaveLength(0);
+      expect(onExit).toHaveBeenCalledWith(0);
+    });
+
+    it('clears stderr buffer after traceback to avoid duplication', async () => {
+      await server.start();
+
+      const traceback = [
+        'Traceback (most recent call last):',
+        '  File "/app/main.py", line 1, in <module>',
+        '    import bad',
+        "ModuleNotFoundError: No module named 'bad'",
+      ].join('\n');
+
+      mockChild.stderr.emit('data', Buffer.from(traceback));
+      vi.mocked(onLog).mockClear();
+
+      // Now exit — stderr buffer should be empty since traceback cleared it
+      mockChild.emit('exit', 1);
+
+      const errorCalls = (onLog as ReturnType<typeof vi.fn>).mock.calls.filter((c: unknown[]) => c[0] === 'error');
+      expect(errorCalls).toHaveLength(0);
+      expect(onExit).toHaveBeenCalledWith(1);
+    });
+  });
 });
diff --git a/src/cli/operations/dev/dev-server.ts b/src/cli/operations/dev/dev-server.ts
@@ -26,8 +26,30 @@ export interface SpawnConfig {
  * Handles process spawning, output parsing, and lifecycle management.
  * Subclasses implement prepare() and getSpawnConfig() for mode-specific behavior.
  */
+/** Keep the last 20 stderr lines so we can surface them if the process crashes.
+ *  20 lines is enough to capture a typical Python traceback or error context
+ *  without accumulating unbounded memory for long-running servers. */
+const STDERR_BUFFER_SIZE = 20;
+
+/** Paths that indicate internal framework frames (not user code) */
+const INTERNAL_FRAME_PATTERNS = [
+  '/site-packages/',
+  '<frozen ',
+  '/multiprocessing/',
+  '/asyncio/',
+  '/concurrent/',
+  '/importlib/',
+];
+
+function isInternalFrame(line: string): boolean {
+  return INTERNAL_FRAME_PATTERNS.some(p => line.includes(p));
+}
+
 export abstract class DevServer {
   protected child: ChildProcess | null = null;
+  private recentStderr: string[] = [];
+  private inTraceback = false;
+  private tracebackBuffer: string[] = [];
 
   constructor(
     protected readonly config: DevConfig,
@@ -71,6 +93,41 @@ export abstract class DevServer {
   /** Returns the command, args, cwd, and environment for the child process. */
   protected abstract getSpawnConfig(): SpawnConfig;
 
+  /**
+   * Emit a filtered Python traceback: only user code frames and the exception line.
+   * Internal frames (site-packages, frozen modules, asyncio, etc.) are stripped out.
+   */
+  private emitFilteredTraceback(onLog: (level: LogLevel, message: string) => void): void {
+    const buf = this.tracebackBuffer;
+    if (buf.length === 0) return;
+
+    // The last line is the exception (e.g., "ModuleNotFoundError: ...")
+    const exceptionLine = buf[buf.length - 1]!;
+
+    // Collect user-code frames: a "File ..." line followed by its code line.
+    // Frames come in pairs: "  File "path", line N, in func" + "    code_line"
+    const userFrames: string[] = [];
+    for (let i = 0; i < buf.length - 1; i++) {
+      const frameLine = buf[i]!;
+      const trimmed = frameLine.trimStart();
+      if (trimmed.startsWith('File ') && !isInternalFrame(frameLine)) {
+        userFrames.push(frameLine);
+        // Include the next line (source code) if it exists and is indented
+        const nextLine = buf[i + 1];
+        if (nextLine && nextLine.startsWith(' ') && !nextLine.trimStart().startsWith('File ')) {
+          userFrames.push(nextLine);
+        }
+      }
+    }
+
+    if (userFrames.length > 0) {
+      for (const frame of userFrames) {
+        onLog('error', frame);
+      }
+    }
+    onLog('error', exceptionLine);
+  }
+
   /** Attach stdout/stderr/error/exit handlers to the child process. */
   private attachHandlers(): void {
     const { onLog, onExit } = this.options.callbacks;
@@ -88,6 +145,31 @@ export abstract class DevServer {
       if (!output) return;
       for (const line of output.split('\n')) {
         if (!line) continue;
+        // Buffer recent stderr for crash context
+        this.recentStderr.push(line);
+        if (this.recentStderr.length > STDERR_BUFFER_SIZE) {
+          this.recentStderr.shift();
+        }
+        // Detect Python traceback blocks: buffer all lines, then emit a
+        // filtered version showing only user code frames + the exception.
+        if (line.startsWith('Traceback (most recent call last)')) {
+          this.inTraceback = true;
+          this.tracebackBuffer = [];
+        }
+        if (this.inTraceback) {
+          this.tracebackBuffer.push(line);
+          const isStackFrame = line.startsWith(' ') || line.startsWith('File ');
+          const isTracebackHeader = line.startsWith('Traceback ');
+          if (!isStackFrame && !isTracebackHeader) {
+            // Traceback ended — emit filtered summary and clear the
+            // stderr buffer so these lines aren't re-emitted on exit.
+            this.emitFilteredTraceback(onLog);
+            this.inTraceback = false;
+            this.tracebackBuffer = [];
+            this.recentStderr = [];
+          }
+          continue;
+        }
         const lower = line.toLowerCase();
         if (lower.includes('warning')) onLog('warn', line);
         else if (lower.includes('error')) onLog('error', line);
@@ -100,6 +182,14 @@ export abstract class DevServer {
       onExit(1);
     });
 
-    this.child?.on('exit', code => onExit(code));
+    this.child?.on('exit', code => {
+      if (code !== 0 && code !== null && this.recentStderr.length > 0) {
+        for (const line of this.recentStderr) {
+          onLog('error', line);
+        }
+        this.recentStderr = [];
+      }
+      onExit(code);
+    });
   }
 }
diff --git a/src/cli/operations/dev/index.ts b/src/cli/operations/dev/index.ts
@@ -10,4 +10,4 @@ export {
 
 export { getDevConfig, getDevSupportedAgents, getAgentPort, loadProjectConfig, type DevConfig } from './config';
 
-export { invokeAgent, invokeAgentStreaming } from './invoke';
+export { ConnectionError, ServerError, invokeAgent, invokeAgentStreaming } from './invoke';
diff --git a/src/cli/operations/dev/invoke.ts b/src/cli/operations/dev/invoke.ts
@@ -1,3 +1,22 @@
+/** Error thrown when the dev server returns a non-OK HTTP response. */
+export class ServerError extends Error {
+  constructor(
+    public readonly statusCode: number,
+    body: string
+  ) {
+    super(body || `Server returned ${statusCode}`);
+    this.name = 'ServerError';
+  }
+}
+
+/** Error thrown when the connection to the dev server fails. */
+export class ConnectionError extends Error {
+  constructor(cause: Error) {
+    super(cause.message);
+    this.name = 'ConnectionError';
+  }
+}
+
 /** Logger interface for SSE events and error logging */
 export interface SSELogger {
   logSSEEvent(rawLine: string): void;
@@ -100,6 +119,11 @@ export async function* invokeAgentStreaming(
         body: JSON.stringify({ prompt: msg }),
       });
 
+      if (!res.ok) {
+        const body = await res.text();
+        throw new ServerError(res.status, body);
+      }
+
       if (!res.body) {
         yield '(empty response)';
         return;
@@ -168,6 +192,12 @@ export async function* invokeAgentStreaming(
 
       return;
     } catch (err) {
+      // Re-throw ServerError directly — no retries for HTTP errors
+      if (err instanceof ServerError) {
+        logger?.log?.('error', `Server error (${err.statusCode}): ${err.message}`);
+        throw err;
+      }
+
       lastError = err instanceof Error ? err : new Error(String(err));
       const isConnectionError = lastError.message.includes('fetch') || lastError.message.includes('ECONNREFUSED');
 
@@ -188,8 +218,8 @@ export async function* invokeAgentStreaming(
   }
 
   // Log final failure after all retries exhausted with full details
-  const finalError = lastError ?? new Error('Failed to connect to dev server after retries');
-  logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.stack ?? finalError.message}`);
+  const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to dev server after retries'));
+  logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
   throw finalError;
 }
 
@@ -223,6 +253,11 @@ export async function invokeAgent(portOrOptions: number | InvokeOptions, message
         body: JSON.stringify({ prompt: msg }),
       });
 
+      if (!res.ok) {
+        const body = await res.text();
+        throw new ServerError(res.status, body);
+      }
+
       const text = await res.text();
       if (!text) {
         return '(empty response)';
@@ -236,6 +271,12 @@ export async function invokeAgent(portOrOptions: number | InvokeOptions, message
       // Handle plain JSON response (non-streaming frameworks)
       return extractResult(text);
     } catch (err) {
+      // Re-throw ServerError directly — no retries for HTTP errors
+      if (err instanceof ServerError) {
+        logger?.log?.('error', `Server error (${err.statusCode}): ${err.message}`);
+        throw err;
+      }
+
       lastError = err instanceof Error ? err : new Error(String(err));
       const isConnectionError = lastError.message.includes('fetch') || lastError.message.includes('ECONNREFUSED');
 
@@ -256,7 +297,7 @@ export async function invokeAgent(portOrOptions: number | InvokeOptions, message
   }
 
   // Log final failure after all retries exhausted with full details
-  const finalError = lastError ?? new Error('Failed to connect to dev server after retries');
-  logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.stack ?? finalError.message}`);
+  const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to dev server after retries'));
+  logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
   throw finalError;
 }
diff --git a/src/cli/tui/hooks/useDevServer.ts b/src/cli/tui/hooks/useDevServer.ts
diff --git a/src/cli/tui/screens/dev/DevScreen.tsx b/src/cli/tui/screens/dev/DevScreen.tsx

Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,4 @@ export {`
`10`	`10`
`11`	`11`	`export { getDevConfig, getDevSupportedAgents, getAgentPort, loadProjectConfig, type DevConfig } from './config';`
`12`	`12`
`13`		`-export { invokeAgent, invokeAgentStreaming } from './invoke';`
	`13`	`+export { ConnectionError, ServerError, invokeAgent, invokeAgentStreaming } from './invoke';`