Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions src/cli/operations/dev/__tests__/dev-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
92 changes: 91 additions & 1 deletion src/cli/operations/dev/dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
});
}
}
2 changes: 1 addition & 1 deletion src/cli/operations/dev/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
49 changes: 45 additions & 4 deletions src/cli/operations/dev/invoke.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');

Expand All @@ -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;
}

Expand Down Expand Up @@ -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)';
Expand All @@ -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');

Expand All @@ -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;
}
Loading
Loading