Fix console logging and message routing

Console Logging Improvements:
- Changed ConsoleRuntimeProvider to send logs immediately instead of batching
- Track pending send promises and await them in onCompleted callback
- Ensures REPL gets all logs before execution-complete
- Enables real-time console logging for HTML artifacts

Message Routing Fixes:
- Remove "handled" concept from message routing - broadcast all messages to all providers/consumers
- Change handleMessage return type from Promise<boolean> to Promise<void>
- Add debug logging to RuntimeMessageRouter to trace message flow
- Fix duplicate error logging (window error handler now only tracks errors, doesn't log them)

Output Formatting Consistency:
- Remove [LOG], [ERROR] prefixes from console output in both tools
- Show console logs before error messages
- Use "=> value" format for return values in both javascript-repl and browser-javascript
- Remove duplicate "Error:" prefix and extra formatting

Bug Fixes:
- Fix race condition where execution-complete arrived before console logs
- Fix ConsoleRuntimeProvider blocking execution-complete from reaching consumers
- Remove duplicate console log collection from SandboxedIframe
- Fix return value capture by wrapping user code in async function
This commit is contained in:
Mario Zechner 2025-10-09 23:20:14 +02:00
parent 38aaaee968
commit 4ac774cbbb
9 changed files with 88 additions and 114 deletions

View file

@ -221,14 +221,7 @@ export class ChatPanel extends LitElement {
${Badge(html` ${Badge(html`
<span class="inline-flex items-center gap-1"> <span class="inline-flex items-center gap-1">
<span>${i18n("Artifacts")}</span> <span>${i18n("Artifacts")}</span>
${ <span class="text-[10px] leading-none bg-primary-foreground/20 text-primary-foreground rounded px-1 font-mono tabular-nums">${this.artifactCount}</span>
this.artifactCount > 1
? html`<span
class="text-[10px] leading-none bg-primary-foreground/20 text-primary-foreground rounded px-1 font-mono tabular-nums"
>${this.artifactCount}</span
>`
: ""
}
</span> </span>
`)} `)}
</button> </button>

View file

@ -163,42 +163,37 @@ export class SandboxIframe extends LitElement {
throw new Error("Execution aborted"); throw new Error("Execution aborted");
} }
providers = [new ConsoleRuntimeProvider(), ...providers]; const consoleProvider = new ConsoleRuntimeProvider();
providers = [consoleProvider, ...providers];
RUNTIME_MESSAGE_ROUTER.registerSandbox(sandboxId, providers, consumers); RUNTIME_MESSAGE_ROUTER.registerSandbox(sandboxId, providers, consumers);
const logs: Array<{ type: string; text: string }> = [];
const files: SandboxFile[] = []; const files: SandboxFile[] = [];
let completed = false; let completed = false;
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
// 4. Create execution consumer for lifecycle messages // 4. Create execution consumer for lifecycle messages
const executionConsumer: MessageConsumer = { const executionConsumer: MessageConsumer = {
async handleMessage(message: any): Promise<boolean> { async handleMessage(message: any): Promise<void> {
if (message.type === "console") { if (message.type === "file-returned") {
logs.push({
type: message.method === "error" ? "error" : "log",
text: message.text,
});
return true;
} else if (message.type === "file-returned") {
files.push({ files.push({
fileName: message.fileName, fileName: message.fileName,
content: message.content, content: message.content,
mimeType: message.mimeType, mimeType: message.mimeType,
}); });
return true;
} else if (message.type === "execution-complete") { } else if (message.type === "execution-complete") {
completed = true; completed = true;
cleanup(); cleanup();
resolve({ success: true, console: logs, files, returnValue: message.returnValue }); resolve({
return true; success: true,
console: consoleProvider.getLogs(),
files,
returnValue: message.returnValue,
});
} else if (message.type === "execution-error") { } else if (message.type === "execution-error") {
completed = true; completed = true;
cleanup(); cleanup();
resolve({ success: false, console: logs, error: message.error, files }); resolve({ success: false, console: consoleProvider.getLogs(), error: message.error, files });
return true;
} }
return false;
}, },
}; };
@ -232,7 +227,7 @@ export class SandboxIframe extends LitElement {
cleanup(); cleanup();
resolve({ resolve({
success: false, success: false,
console: logs, console: consoleProvider.getLogs(),
error: { message: "Execution timeout (30s)", stack: "" }, error: { message: "Execution timeout (30s)", stack: "" },
files, files,
}); });
@ -347,7 +342,7 @@ export class SandboxIframe extends LitElement {
await window.complete(null, returnValue); await window.complete(null, returnValue);
} catch (error) { } catch (error) {
// Call completion callbacks before complete() (error path) // Call completion callbacks before complete() (error path)
if (window.__completionCallbacks && window.__completionCallbacks.length > 0) { if (window.__completionCallbacks && window.__completionCallbacks.length > 0) {
try { try {

View file

@ -143,9 +143,9 @@ export class ArtifactsRuntimeProvider implements SandboxRuntimeProvider {
}; };
} }
async handleMessage(message: any, respond: (response: any) => void): Promise<boolean> { async handleMessage(message: any, respond: (response: any) => void): Promise<void> {
if (message.type !== "artifact-operation") { if (message.type !== "artifact-operation") {
return false; return;
} }
const { action, filename, content, mimeType } = message; const { action, filename, content, mimeType } = message;
@ -224,11 +224,8 @@ export class ArtifactsRuntimeProvider implements SandboxRuntimeProvider {
default: default:
respond({ success: false, error: `Unknown artifact action: ${action}` }); respond({ success: false, error: `Unknown artifact action: ${action}` });
} }
return true;
} catch (error: any) { } catch (error: any) {
respond({ success: false, error: error.message }); respond({ success: false, error: error.message });
return true;
} }
} }

View file

@ -25,7 +25,7 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
getRuntime(): (sandboxId: string) => void { getRuntime(): (sandboxId: string) => void {
return (_sandboxId: string) => { return (_sandboxId: string) => {
// Console capture with immediate send + completion batch pattern // Console capture with immediate send pattern
const originalConsole = { const originalConsole = {
log: console.log, log: console.log,
error: console.error, error: console.error,
@ -33,8 +33,8 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
info: console.info, info: console.info,
}; };
// Collect logs locally, send at completion // Track pending send promises to wait for them in onCompleted
const collectedLogs: Array<{ method: string; text: string; args: any[] }> = []; const pendingSends: Promise<any>[] = [];
["log", "error", "warn", "info"].forEach((method) => { ["log", "error", "warn", "info"].forEach((method) => {
(console as any)[method] = (...args: any[]) => { (console as any)[method] = (...args: any[]) => {
@ -48,29 +48,30 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
}) })
.join(" "); .join(" ");
// Collect log for batch send at completion
collectedLogs.push({ method, text, args });
// Always log locally too // Always log locally too
(originalConsole as any)[method].apply(console, args); (originalConsole as any)[method].apply(console, args);
// Send immediately and track the promise
if ((window as any).sendRuntimeMessage) {
const sendPromise = (window as any)
.sendRuntimeMessage({
type: "console",
method,
text,
args,
})
.catch(() => {});
pendingSends.push(sendPromise);
}
}; };
}); });
// Register completion callback to send all collected logs // Register completion callback to wait for all pending sends
if ((window as any).onCompleted) { if ((window as any).onCompleted) {
(window as any).onCompleted(async (_success: boolean) => { (window as any).onCompleted(async (_success: boolean) => {
// Send all collected logs // Wait for all pending console sends to complete
if (collectedLogs.length > 0 && (window as any).sendRuntimeMessage) { if (pendingSends.length > 0) {
await Promise.all( await Promise.all(pendingSends);
collectedLogs.map((logEntry) =>
(window as any).sendRuntimeMessage({
type: "console",
method: logEntry.method,
text: logEntry.text,
args: logEntry.args,
}),
),
);
} }
}); });
} }
@ -78,7 +79,8 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
// Track errors for HTML artifacts // Track errors for HTML artifacts
let lastError: { message: string; stack: string } | null = null; let lastError: { message: string; stack: string } | null = null;
// Error handlers // Error handlers - track errors but don't log them
// (they'll be shown via execution-error message)
window.addEventListener("error", (e) => { window.addEventListener("error", (e) => {
const text = const text =
(e.error?.stack || e.message || String(e)) + " at line " + (e.lineno || "?") + ":" + (e.colno || "?"); (e.error?.stack || e.message || String(e)) + " at line " + (e.lineno || "?") + ":" + (e.colno || "?");
@ -87,16 +89,6 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
message: e.error?.message || e.message || String(e), message: e.error?.message || e.message || String(e),
stack: e.error?.stack || text, stack: e.error?.stack || text,
}; };
if ((window as any).sendRuntimeMessage) {
(window as any)
.sendRuntimeMessage({
type: "console",
method: "error",
text,
})
.catch(() => {});
}
}); });
window.addEventListener("unhandledrejection", (e) => { window.addEventListener("unhandledrejection", (e) => {
@ -106,16 +98,6 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
message: e.reason?.message || String(e.reason) || "Unhandled promise rejection", message: e.reason?.message || String(e.reason) || "Unhandled promise rejection",
stack: e.reason?.stack || text, stack: e.reason?.stack || text,
}; };
if ((window as any).sendRuntimeMessage) {
(window as any)
.sendRuntimeMessage({
type: "console",
method: "error",
text,
})
.catch(() => {});
}
}); });
// Expose complete() method for user code to call // Expose complete() method for user code to call
@ -143,7 +125,7 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
}; };
} }
async handleMessage(message: any, respond: (response: any) => void): Promise<boolean> { async handleMessage(message: any, respond: (response: any) => void): Promise<void> {
if (message.type === "console") { if (message.type === "console") {
// Collect console output // Collect console output
this.logs.push({ this.logs.push({
@ -160,10 +142,7 @@ export class ConsoleRuntimeProvider implements SandboxRuntimeProvider {
}); });
// Acknowledge receipt // Acknowledge receipt
respond({ success: true }); respond({ success: true });
return true;
} }
return false;
} }
/** /**

View file

@ -77,20 +77,17 @@ export class FileDownloadRuntimeProvider implements SandboxRuntimeProvider {
}; };
} }
async handleMessage(message: any, respond: (response: any) => void): Promise<boolean> { async handleMessage(message: any, respond: (response: any) => void): Promise<void> {
if (message.type !== "file-returned") { if (message.type === "file-returned") {
return false; // Collect file for caller
this.files.push({
fileName: message.fileName,
content: message.content,
mimeType: message.mimeType,
});
respond({ success: true });
} }
// Collect file for caller
this.files.push({
fileName: message.fileName,
content: message.content,
mimeType: message.mimeType,
});
respond({ success: true });
return true;
} }
/** /**

View file

@ -9,9 +9,9 @@ declare const chrome: any;
export interface MessageConsumer { export interface MessageConsumer {
/** /**
* Handle a message from a sandbox. * Handle a message from a sandbox.
* @returns true if message was consumed (stops propagation), false otherwise * All consumers receive all messages - decide internally what to handle.
*/ */
handleMessage(message: any): Promise<boolean>; handleMessage(message: any): Promise<void>;
} }
/** /**
@ -59,7 +59,7 @@ export class RuntimeMessageRouter {
// Setup global listener if not already done // Setup global listener if not already done
this.setupListener(); this.setupListener();
console.log("Registered sandbox:", sandboxId); console.log(`Registered sandbox: ${sandboxId}, providers: ${providers.length}, consumers: ${consumers.length}`);
} }
/** /**
@ -132,10 +132,20 @@ export class RuntimeMessageRouter {
const { sandboxId, messageId } = e.data; const { sandboxId, messageId } = e.data;
if (!sandboxId) return; if (!sandboxId) return;
console.log("Router received message for sandbox:", sandboxId, e.data); console.log(
"[ROUTER] Received message for sandbox:",
sandboxId,
"type:",
e.data.type,
"full message:",
e.data,
);
const context = this.sandboxes.get(sandboxId); const context = this.sandboxes.get(sandboxId);
if (!context) return; if (!context) {
console.log("[ROUTER] No context found for sandbox:", sandboxId);
return;
}
// Create respond() function for bidirectional communication // Create respond() function for bidirectional communication
const respond = (response: any) => { const respond = (response: any) => {
@ -151,15 +161,19 @@ export class RuntimeMessageRouter {
}; };
// 1. Try provider handlers first (for bidirectional comm) // 1. Try provider handlers first (for bidirectional comm)
console.log("[ROUTER] Broadcasting to", context.providers.length, "providers");
for (const provider of context.providers) { for (const provider of context.providers) {
if (provider.handleMessage) { if (provider.handleMessage) {
console.log("[ROUTER] Calling provider.handleMessage for", provider.constructor.name);
await provider.handleMessage(e.data, respond); await provider.handleMessage(e.data, respond);
// Don't stop - let consumers also handle the message // Don't stop - let consumers also handle the message
} }
} }
// 2. Broadcast to consumers (one-way messages or lifecycle events) // 2. Broadcast to consumers (one-way messages or lifecycle events)
console.log("[ROUTER] Broadcasting to", context.consumers.size, "consumers");
for (const consumer of context.consumers) { for (const consumer of context.consumers) {
console.log("[ROUTER] Calling consumer.handleMessage");
await consumer.handleMessage(e.data); await consumer.handleMessage(e.data);
// Don't stop - let all consumers see the message // Don't stop - let all consumers see the message
} }
@ -194,17 +208,18 @@ export class RuntimeMessageRouter {
// Route to providers (async) // Route to providers (async)
(async () => { (async () => {
// 1. Try provider handlers first (for bidirectional comm)
for (const provider of context.providers) { for (const provider of context.providers) {
if (provider.handleMessage) { if (provider.handleMessage) {
const handled = await provider.handleMessage(message, respond); await provider.handleMessage(message, respond);
if (handled) return; // Don't stop - let consumers also handle the message
} }
} }
// Broadcast to consumers // 2. Broadcast to consumers (one-way messages or lifecycle events)
for (const consumer of context.consumers) { for (const consumer of context.consumers) {
const consumed = await consumer.handleMessage(message); await consumer.handleMessage(message);
if (consumed) break; // Don't stop - let all consumers see the message
} }
})(); })();

View file

@ -20,13 +20,12 @@ export interface SandboxRuntimeProvider {
/** /**
* Optional message handler for bidirectional communication. * Optional message handler for bidirectional communication.
* Return true if the message was handled, false to let other handlers try. * All providers receive all messages - decide internally what to handle.
* *
* @param message - The message from the sandbox * @param message - The message from the sandbox
* @param respond - Function to send a response back to the sandbox * @param respond - Function to send a response back to the sandbox
* @returns true if message was handled, false otherwise
*/ */
handleMessage?(message: any, respond: (response: any) => void): Promise<boolean>; handleMessage?(message: any, respond: (response: any) => void): Promise<void>;
/** /**
* Optional documentation describing what globals/functions this provider injects. * Optional documentation describing what globals/functions this provider injects.

View file

@ -86,7 +86,7 @@ export class HtmlArtifact extends ArtifactElement {
// Create consumer for console messages // Create consumer for console messages
const consumer: MessageConsumer = { const consumer: MessageConsumer = {
handleMessage: async (message: any): Promise<boolean> => { handleMessage: async (message: any): Promise<void> => {
if (message.type === "console") { if (message.type === "console") {
// Create new array reference for Lit reactivity // Create new array reference for Lit reactivity
this.logs = [ this.logs = [
@ -97,9 +97,7 @@ export class HtmlArtifact extends ArtifactElement {
}, },
]; ];
this.requestUpdate(); // Re-render to show console this.requestUpdate(); // Re-render to show console
return true;
} }
return false;
}, },
}; };

View file

@ -44,25 +44,26 @@ export async function executeJavaScript(
// Remove the sandbox iframe after execution // Remove the sandbox iframe after execution
sandbox.remove(); sandbox.remove();
// Return plain text output
if (!result.success) {
// Return error as plain text
return {
output: `Error: ${result.error?.message || "Unknown error"}\n${result.error?.stack || ""}`,
};
}
// Build plain text response // Build plain text response
let output = ""; let output = "";
// Add console output - result.console contains { type: string, text: string } from sandbox.js // Add console output - result.console contains { type: string, text: string } from sandbox.js
if (result.console && result.console.length > 0) { if (result.console && result.console.length > 0) {
for (const entry of result.console) { for (const entry of result.console) {
const prefix = entry.type === "error" ? "[ERROR]" : ""; output += entry.text + "\n";
output += (prefix ? `${prefix} ` : "") + entry.text + "\n";
} }
} }
// Add error if execution failed
if (!result.success) {
if (output) output += "\n";
output += `Error: ${result.error?.message || "Unknown error"}\n${result.error?.stack || ""}`;
return {
output: output.trim(),
};
}
// Add return value if present // Add return value if present
if (result.returnValue !== undefined) { if (result.returnValue !== undefined) {
if (output) output += "\n"; if (output) output += "\n";