-
Notifications
You must be signed in to change notification settings - Fork 146
Implementation for debug-isolated flag and streaming func CLI output #4765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for streaming terminal output from Azure Functions host tasks and improves debugging for .NET isolated functions. The main purpose is to enable real-time access to process output and automatically extract process IDs from terminal output when using the --dotnet-isolated-debug flag.
Key changes:
- Introduces an async string stream utility for capturing and consuming terminal output
- Integrates streaming into the function task lifecycle and exposes it through the API
- Adds support for extracting process IDs from JSON-formatted terminal output in .NET isolated debug mode
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/stream.ts | New async string stream utility for managing terminal output |
| src/funcCoreTools/funcHostTask.ts | Integrates stream handler into task lifecycle and ensures cleanup on task termination |
| src/debug/FuncTaskProvider.ts | Adds support for passing additional arguments to func tasks |
| src/commands/pickFuncProcess.ts | Implements terminal output streaming and PID extraction for debug mode |
| package.json | Enables terminalDataWriteEvent API and adds args property to task definition |
src/commands/pickFuncProcess.ts
Outdated
| vscode.window.onDidWriteTerminalData(async (event: vscode.TerminalDataWriteEvent) => { | ||
| const terminal = vscode.window.terminals.find(t => funcTask.name === t.name); | ||
| if (event.terminal === terminal) { | ||
| taskInfo.streamHandler.write(event.data); | ||
| } | ||
| }); |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event listener created by onDidWriteTerminalData is never disposed, causing a resource leak. Store the returned Disposable and dispose it when the task completes or errors.
Co-authored-by: Copilot <[email protected]>
…ft/vscode-azurefunctions into nat/fileBasedPickProcess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
src/commands/pickFuncProcess.ts
Outdated
| const funcPort: string = await getFuncPortFromTaskOrProject(context, funcTask, workspaceFolder); | ||
| let statusRequestTimeout: number = intervalMs; | ||
| const maxTime: number = Date.now() + timeoutInSeconds * 1000; | ||
| const debugModeOn = funcTask.name.includes('--dotnet-isolated-debug') && funcTask.name.includes('--enable-json-output'); |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking for command-line flags in the task name is fragile and unclear. Task names are meant for display purposes. Consider checking the actual command line or task definition properties instead.
src/commands/pickFuncProcess.ts
Outdated
| async function setEventPidByJsonOutput(taskInfo: IRunningFuncTask, taskName: string): Promise<vscode.Disposable> { | ||
| const setPidByJsonOutputListener = vscode.window.onDidWriteTerminalData(async (event: vscode.TerminalDataWriteEvent) => { | ||
| const terminal = vscode.window.terminals.find(t => taskName === t.name); | ||
| if (event.terminal === terminal) { | ||
| if (event.data.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | ||
| const matches = event.data.match(/"workerProcessId"\s*:\s*(\d+)/); | ||
| if (matches && matches.length > 1) { | ||
| taskInfo.processId = Number(matches[1]); | ||
| setPidByJsonOutputListener.dispose(); | ||
| } | ||
| } | ||
| } | ||
| }); |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The listener mutates taskInfo.processId without synchronization. If terminal data arrives concurrently while the main loop in startFuncTask checks taskInfo.processId !== parentPid, there's a potential race condition where the PID could be updated between the check and the return statement.
src/funcCoreTools/funcHostTask.ts
Outdated
| const outputReader = vscode.window.onDidWriteTerminalData(async (event: vscode.TerminalDataWriteEvent) => { | ||
| const terminal = vscode.window.terminals.find(t => terminalName === t.name); | ||
| if (event.terminal === terminal) { | ||
| runningFuncTask.streamHandler.write(event.data); | ||
| } | ||
| }); |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable runningFuncTask is referenced before it's declared on line 107. This will cause a ReferenceError at runtime. Move the declaration of runningFuncTask before the outputReader initialization.
src/commands/pickFuncProcess.ts
Outdated
| const setPidByJsonOutputListener = vscode.window.onDidWriteTerminalData(async (event: vscode.TerminalDataWriteEvent) => { | ||
| const terminal = vscode.window.terminals.find(t => taskName === t.name); | ||
| if (event.terminal === terminal) { | ||
| if (event.data.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a hardcoded JSON fragment string for matching is fragile. JSON can have varying whitespace. Consider parsing the output as JSON or using a more robust pattern match that accounts for whitespace variations.
Co-authored-by: Copilot <[email protected]>
…ft/vscode-azurefunctions into nat/fileBasedPickProcess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
src/funcCoreTools/funcHostTask.ts
Outdated
| const terminalEventReader = vscode.window.onDidStartTerminalShellExecution(async (terminalShellExecEvent) => { | ||
| /** | ||
| * NOTE: there is no reliable way to link a terminal to a task due to the name and PID not updating in real time, | ||
| * so just keep updating to the latest event since the func task and its dependencies run in the same | ||
| * terminal (the terminal that we want to output) | ||
| * New tasks will create new `terminalShellExecutionEvents`, so we don't need to worry about picking up output from other terminals | ||
| * */ | ||
| latestTerminalShellExecutionEvent = terminalShellExecEvent; | ||
| }); |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global latestTerminalShellExecutionEvent variable creates a race condition when multiple func tasks start concurrently. If two tasks start simultaneously, both will share the same terminalEventReader listener, but latestTerminalShellExecutionEvent may point to the wrong terminal. Consider storing the terminal event directly in the task object or using a Map keyed by task execution to avoid cross-contamination between concurrent tasks.
src/funcCoreTools/funcHostTask.ts
Outdated
| terminalEventReader, | ||
| stream: latestTerminalShellExecutionEvent?.execution.read() |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stream may be assigned from an unrelated terminal due to the global latestTerminalShellExecutionEvent variable. Since the terminalEventReader listener updates this global asynchronously and there's no synchronization, the stream captured at line 116 might belong to a different task that fired its event between lines 104 and 116, especially under concurrent task execution.
src/funcCoreTools/funcHostTask.ts
Outdated
| let latestTerminalShellExecutionEvent: vscode.TerminalShellExecutionStartEvent | undefined; | ||
| export function registerFuncHostTaskEvents(): void { | ||
| registerEvent('azureFunctions.onDidStartTask', vscode.tasks.onDidStartTaskProcess, async (context: IActionContext, e: vscode.TaskProcessStartEvent) => { | ||
| const terminalEventReader = vscode.window.onDidStartTerminalShellExecution(async (terminalShellExecEvent) => { |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new terminal event listener is created for every task start event, but this listener captures events globally across all terminals. This creates unnecessary overhead with multiple event handlers for the same global events. Consider using a single global listener that filters by task context, or dispose of the listener once the relevant event is captured.
| for await (const chunk of taskInfo.stream) { | ||
| if (chunk.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | ||
| const matches = chunk.match(/"workerProcessId"\s*:\s*(\d+)/); | ||
| if (matches && matches.length > 1) { | ||
| return Number(matches[1]); | ||
| } | ||
| } | ||
| } | ||
| return; |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The for await loop will consume the entire stream and never exit if the expected JSON output is not found, causing the function to hang indefinitely. The stream reading should have a timeout or break condition, and the loop at line 156 in startFuncTask should handle this case to avoid waiting forever.
| for await (const chunk of taskInfo.stream) { | |
| if (chunk.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | |
| const matches = chunk.match(/"workerProcessId"\s*:\s*(\d+)/); | |
| if (matches && matches.length > 1) { | |
| return Number(matches[1]); | |
| } | |
| } | |
| } | |
| return; | |
| const TIMEOUT_MS = 10000; // 10 seconds | |
| let timeoutHandle: NodeJS.Timeout; | |
| return await Promise.race([ | |
| (async () => { | |
| for await (const chunk of taskInfo.stream) { | |
| if (chunk.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | |
| const matches = chunk.match(/"workerProcessId"\s*:\s*(\d+)/); | |
| if (matches && matches.length > 1) { | |
| clearTimeout(timeoutHandle); | |
| return Number(matches[1]); | |
| } | |
| } | |
| } | |
| return undefined; | |
| })(), | |
| new Promise<number | undefined>(resolve => { | |
| timeoutHandle = setTimeout(() => { | |
| resolve(undefined); | |
| }, TIMEOUT_MS); | |
| }) | |
| ]); |
| if (chunk.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | ||
| const matches = chunk.match(/"workerProcessId"\s*:\s*(\d+)/); | ||
| if (matches && matches.length > 1) { | ||
| return Number(matches[1]); | ||
| } | ||
| } |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded JSON string fragment with specific formatting assumptions is brittle. JSON output may have varying whitespace, and this check will fail if the format changes slightly (e.g., no space after :). Consider parsing the chunk as JSON and checking the structure programmatically, or use a more flexible pattern that accommodates whitespace variations.
| if (chunk.includes(`{ "name":"dotnet-worker-startup", "workerProcessId" :`)) { | |
| const matches = chunk.match(/"workerProcessId"\s*:\s*(\d+)/); | |
| if (matches && matches.length > 1) { | |
| return Number(matches[1]); | |
| } | |
| } | |
| let obj: unknown; | |
| try { | |
| obj = JSON.parse(chunk); | |
| } catch { | |
| continue; // Not valid JSON, skip this chunk | |
| } | |
| if ( | |
| typeof obj === 'object' && | |
| obj !== null && | |
| (obj as any).name === "dotnet-worker-startup" && | |
| typeof (obj as any).workerProcessId === "number" | |
| ) { | |
| return (obj as any).workerProcessId; | |
| } |
| const args = (definition?.args || []) as string[]; | ||
| if (args.length > 0) { | ||
| command = `${command} ${args.join(' ')}`; | ||
| } |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command arguments from task definitions are concatenated directly into shell commands without sanitization or escaping. If args contain shell metacharacters (e.g., ;, |, $(), backticks), this could lead to command injection. Consider using proper shell escaping or switching to an array-based execution approach where arguments are passed separately from the command.
Fixes #4745
Fixes #4746
This pull request introduces support for streaming terminal output from Azure Functions host tasks, enabling consumers of the API to access real-time output from running processes.
It adds a new async string stream utility, integrates this stream into the function task lifecycle, and exposes the stream through the API.Additionally, it improves handling for .NET isolated debug scenarios by extracting process IDs from terminal output if the flag is set.
Minor updates were made to the package configuration to support these features, including func tasks now support the
argsproperty since --debug-isolate is ugly to write all in the command.Configuration Updates* Added theenabledApiProposalssection topackage.jsonto enable theterminalDataWriteEventAPI, which is required for capturing terminal output.EDIT: To read the stream, you can run this snippet: