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
2 changes: 2 additions & 0 deletions apps/vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## 1.135.0 (Unreleased)

- In Positron, running a cell that raises an error no longer results in an error toast message (<https://github.com/posit-dev/positron/issues/9845>).

## 1.134.0 (Release on 2026-06-22)

- The "Preview" and "Preview Format..." commands now show in the Positron Notebook Editor overflow menu (<https://github.com/quarto-dev/quarto/pull/1001>).
Expand Down
4 changes: 2 additions & 2 deletions apps/vscode/src/host/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ export interface ExtensionHost {

}

export function extensionHost(): ExtensionHost {
export function extensionHost(outputChannel?: vscode.LogOutputChannel): ExtensionHost {
if (tryAcquirePositronApi()) {
return positronExtensionHost();
return positronExtensionHost(outputChannel);
} else {
return defaultExtensionHost();
}
Expand Down
60 changes: 47 additions & 13 deletions apps/vscode/src/host/positron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function isInlineOutputEnabled(): boolean {
.get<boolean>("enabled", false);
}

export function positronExtensionHost(): ExtensionHost {
export function positronExtensionHost(outputChannel?: vscode.LogOutputChannel): ExtensionHost {
return {
// supported executable languages (we delegate to the default for langugaes
// w/o runtimes so we support all languages)
Expand Down Expand Up @@ -68,18 +68,52 @@ export function positronExtensionHost(): ExtensionHost {
const callback = async () => {
for (let i = 0; i < blocks.length; i++) {
const metadata = executionMetadata?.[i];
await runtime.executeCode(
language, // The language ID
blocks[i], // The code string to execute
false, // Whether to focus the console
true, // Whether to allow incomplete code to run
undefined, // The execution mode
undefined, // The error behavior
undefined, // An optional observer
undefined, // The specific session ID in which to execute
editorUri, // The document URI
metadata
);

// Track whether the runtime itself reported the failure. A
// failure of the running code (a runtime error or syntax
// error in the cell), an interrupt, or the session exiting are
// all delivered through the execution observer's `onFailed`
// callback before `executeCode` rejects. These are already
// surfaced in the console, so we don't want to surface them
// again. A rejection *without* `onFailed` having fired means
// the code couldn't be submitted to the runtime at all (e.g.
// no runtime is registered for the language), which is a
// genuine problem worth reporting.
let runtimeFailure = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic depends on onFailed having already fired (synchronously setting runtimeFailure) by the time we reach this catch. The ExecutionObserver docs only guarantee that one of onCompleted/onFailed is called, not their ordering relative to the executeCode promise rejecting. If that ordering ever changed or raced, a runtime error would be misclassified as a submission failure and we'd surface the popup again. Just noting the dependency for future readers; the failure mode is only a graceful regression to the old behavior, so nothing to change here IMO.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a little kludgy! I think a more complete fix might be to throw specific structured error classes from Positron to distinguish "can't run code at all" errors from "code ran but it failed" errors.


try {
await runtime.executeCode(
language, // The language ID
blocks[i], // The code string to execute
false, // Whether to focus the console
true, // Whether to allow incomplete code to run
undefined, // The execution mode
undefined, // The error behavior
{ onFailed: () => { runtimeFailure = true; } }, // An execution observer
undefined, // The specific session ID in which to execute
editorUri, // The document URI
metadata
);
} catch (err) {
const message = err instanceof Error ? err.message : JSON.stringify(err);

if (!runtimeFailure) {
// The code couldn't be submitted to the runtime. Log it
// and let it propagate so the user finds out.
outputChannel?.error(`Failed to execute ${language} cell: ${message}`);
throw err;
}

// The executed code raised an error (or was interrupted).
// It's already reported in the console, so log it for the
// record but don't let it propagate to the command handler,
// which would surface it again as a notification popup.
// https://github.com/posit-dev/positron/issues/9845
outputChannel?.debug(`Error executing ${language} cell: ${message}`);

// Stop executing any subsequent blocks since one failed.
break;
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion apps/vscode/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<Quarto
outputChannel.info("Activating Quarto extension.");

// create extension host
const host = extensionHost();
const host = extensionHost(outputChannel);

// create markdown engine
const engine = new MarkdownEngine();
Expand Down
Loading