Skip to content

Commit

Permalink
Better handle markup rendering errors in noetebooks (#161153)
Browse files Browse the repository at this point in the history 
Better handle markup rendering errors

Show an error to users if rendering of markup items fail. Previously when a renderer fails to load or render, we would not show any user facing errors (and would sometimes prevent the entire notebook from loading)
  • Loading branch information
@mjbvz
mjbvz authored Sep 19, 2022
1 parent 2ae5bc6 commit 3f92eb6
Showing 1 changed file with 81 additions and 48 deletions.
129 changes: 81 additions & 48 deletions src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,18 @@ async function webviewPreloads(ctx: PreloadContext) {
};
}

function showPreloadErrors(outputNode: HTMLElement, ...errors: readonly Error[]) {
outputNode.innerText = `Error loading preloads:`;
async function renderOutputItem(rendererApi: rendererApi.RendererApi, rendererId: string, item: rendererApi.OutputItem, element: HTMLElement, signal: AbortSignal) {
try {
await rendererApi.renderOutputItem(item, element, signal);
} catch (e) {
if (!signal.aborted) {
showRenderError(`Error rendering output item using '${rendererId}'`, element, e instanceof Error ? [e] : []);
}
}
}

function showRenderError(errorText: string, outputNode: HTMLElement, errors: readonly Error[]) {
outputNode.innerText = errorText;
const errList = document.createElement('ul');
for (const result of errors) {
console.error(result);
Expand Down Expand Up @@ -1087,8 +1097,8 @@ async function webviewPreloads(ctx: PreloadContext) {

case 'html': {
const data = event.data;
outputRunner.enqueue(data.outputId, (state) => {
return viewModel.renderOutputCell(data, state);
outputRunner.enqueue(data.outputId, signal => {
return viewModel.renderOutputCell(data, signal);
});
break;
}
Expand Down Expand Up @@ -1336,27 +1346,32 @@ async function webviewPreloads(ctx: PreloadContext) {
};

const outputRunner = new class {
private readonly outputs = new Map<string, { cancelled: boolean; queue: Promise<unknown> }>();
private readonly outputs = new Map<string, { abort: AbortController; queue: Promise<unknown> }>();

/**
* Pushes the action onto the list of actions for the given output ID,
* ensuring that it's run in-order.
*/
public enqueue(outputId: string, action: (record: { cancelled: boolean }) => unknown) {
public enqueue(outputId: string, action: (cancelSignal: AbortSignal) => unknown) {
const record = this.outputs.get(outputId);
if (!record) {
this.outputs.set(outputId, { cancelled: false, queue: new Promise(r => r(action({ cancelled: false }))) });
const controller = new AbortController();
this.outputs.set(outputId, { abort: controller, queue: new Promise(r => r(action(controller.signal))) });
} else {
record.queue = record.queue.then(r => !record.cancelled && action(record));
record.queue = record.queue.then(r => {
if (!record.abort.signal.aborted) {
return action(record.abort.signal);
}
});
}
}

/**
* Cancels the rendering of all outputs.
*/
public cancelAll() {
for (const record of this.outputs.values()) {
record.cancelled = true;
for (const { abort } of this.outputs.values()) {
abort.abort();
}
this.outputs.clear();
}
Expand All @@ -1367,7 +1382,7 @@ async function webviewPreloads(ctx: PreloadContext) {
public cancelOutput(outputId: string) {
const output = this.outputs.get(outputId);
if (output) {
output.cancelled = true;
output.abort.abort();
this.outputs.delete(outputId);
}
}
Expand Down Expand Up @@ -1458,8 +1473,8 @@ async function webviewPreloads(ctx: PreloadContext) {
}

public async render(info: rendererApi.OutputItem, element: HTMLElement, signal: AbortSignal): Promise<void> {
const renderers = Array.from(this._renderers.values())
.filter(renderer => renderer.data.mimeTypes.includes(info.mime) && !renderer.data.extends);
const renderers = Array.from(this._renderers.entries())
.filter(([_, renderer]) => renderer.data.mimeTypes.includes(info.mime) && !renderer.data.extends);

if (!renderers.length) {
const errorContainer = document.createElement('div');
Expand All @@ -1482,12 +1497,23 @@ async function webviewPreloads(ctx: PreloadContext) {
}

// De-prioritize built-in renderers
renderers.sort((a, b) => +a.data.isBuiltin - +b.data.isBuiltin);
renderers.sort((a, b) => +a[1].data.isBuiltin - +b[1].data.isBuiltin);
const renderer = renderers[0];

const renderer = await renderers[0].load();
if (renderer) {
await renderer.renderOutputItem(info, element, signal);
let renderApi: rendererApi.RendererApi | undefined;
try {
renderApi = await renderer[1].load();
if (!renderApi) {
return;
}
} catch (e) {
if (!signal.aborted) {
showRenderError(`Error loading renderer '${renderer[0]}'`, element, e instanceof Error ? [e] : []);
}
return;
}

await renderOutputItem(renderApi, renderer[0], info, element, signal);
}
}();

Expand Down Expand Up @@ -1610,22 +1636,18 @@ async function webviewPreloads(ctx: PreloadContext) {
}
}

public async renderOutputCell(data: webviewMessages.ICreationRequestMessage, state: { cancelled: boolean }): Promise<void> {
public async renderOutputCell(data: webviewMessages.ICreationRequestMessage, signal: AbortSignal): Promise<void> {
const preloadsAndErrors = await Promise.all<unknown>([
data.rendererId ? renderers.load(data.rendererId) : undefined,
...data.requiredPreloads.map(p => kernelPreloads.waitFor(p.uri)),
].map(p => p?.catch(err => err)));

if (state.cancelled) {
if (signal.aborted) {
return;
}

const cellOutput = this.ensureOutputCell(data.cellId, data.cellTop, false);
const outputNode = cellOutput.createOutputElement(data.outputId, data.outputOffset, data.left, data.cellId);
outputNode.render(data.content, preloadsAndErrors);

// don't hide until after this step so that the height is right
cellOutput.element.style.visibility = data.initiallyHidden ? 'hidden' : 'visible';
await cellOutput.renderOutputElement(data, data.rendererId ?? 'preload', preloadsAndErrors, signal);
}

public ensureOutputCell(cellId: string, cellTop: number, skipCellTopUpdateIfExist: boolean): OutputCell {
Expand Down Expand Up @@ -1927,7 +1949,6 @@ async function webviewPreloads(ctx: PreloadContext) {
}

class OutputCell {

public readonly element: HTMLElement;
private readonly outputElements = new Map</*outputId*/ string, OutputContainer>();

Expand Down Expand Up @@ -1957,15 +1978,23 @@ async function webviewPreloads(ctx: PreloadContext) {
this.outputElements.clear();
}

public createOutputElement(outputId: string, outputOffset: number, left: number, cellId: string): OutputElement {
let outputContainer = this.outputElements.get(outputId);
private createOutputElement(data: webviewMessages.ICreationRequestMessage): OutputElement {
let outputContainer = this.outputElements.get(data.outputId);
if (!outputContainer) {
outputContainer = new OutputContainer(outputId);
outputContainer = new OutputContainer(data.outputId);
this.element.appendChild(outputContainer.element);
this.outputElements.set(outputId, outputContainer);
this.outputElements.set(data.outputId, outputContainer);
}

return outputContainer.createOutputElement(outputId, outputOffset, left, cellId);
return outputContainer.createOutputElement(data.outputId, data.outputOffset, data.left, data.cellId);
}

public async renderOutputElement(data: webviewMessages.ICreationRequestMessage, rendererId: string, preloadsAndErrors: unknown[], signal: AbortSignal) {
const outputElement = this.createOutputElement(data);
await outputElement.render(data.content, rendererId, preloadsAndErrors, signal);

// don't hide until after this step so that the height is right
outputElement.element.style.visibility = data.initiallyHidden ? 'hidden' : 'visible';
}

public clearOutput(outputId: string, rendererId: string | undefined) {
Expand Down Expand Up @@ -2091,7 +2120,11 @@ async function webviewPreloads(ctx: PreloadContext) {

class OutputElement {
public readonly element: HTMLElement;
private _content?: { content: webviewMessages.ICreationContent; preloadsAndErrors: unknown[] };
private _content?: {
content: webviewMessages.ICreationContent;
rendererId: string;
preloadsAndErrors: unknown[];
};
private hasResizeObserver = false;

private renderTaskAbort?: AbortController;
Expand Down Expand Up @@ -2122,34 +2155,34 @@ async function webviewPreloads(ctx: PreloadContext) {
this.renderTaskAbort = undefined;
}

public async render(content: webviewMessages.ICreationContent, preloadsAndErrors: unknown[]) {
public async render(content: webviewMessages.ICreationContent, rendererId: string, preloadsAndErrors: unknown[], signal?: AbortSignal) {
this.renderTaskAbort?.abort();
this.renderTaskAbort = undefined;

this._content = { content, preloadsAndErrors };
this._content = { content, rendererId, preloadsAndErrors };
if (content.type === 0 /* RenderOutputType.Html */) {
const trustedHtml = ttPolicy?.createHTML(content.htmlContent) ?? content.htmlContent;
this.element.innerHTML = trustedHtml as string;
domEval(this.element);
} else if (preloadsAndErrors.some(e => e instanceof Error)) {
const errors = preloadsAndErrors.filter((e): e is Error => e instanceof Error);
showPreloadErrors(this.element, ...errors);
showRenderError(`Error loading preloads`, this.element, errors);
} else {
const rendererApi = preloadsAndErrors[0] as rendererApi.RendererApi;
try {
const item = createOutputItem(this.outputId, content.mimeType, content.metadata, content.valueBytes);
const item = createOutputItem(this.outputId, content.mimeType, content.metadata, content.valueBytes);

const controller = new AbortController();
this.renderTaskAbort = controller;
try {
await rendererApi.renderOutputItem(item, this.element, this.renderTaskAbort.signal);
} finally {
if (this.renderTaskAbort === controller) {
this.renderTaskAbort = undefined;
}
const controller = new AbortController();
this.renderTaskAbort = controller;

// Abort rendering if caller aborts
signal?.addEventListener('abort', () => controller.abort());

try {
await renderOutputItem(rendererApi, rendererId, item, this.element, controller.signal);
} finally {
if (this.renderTaskAbort === controller) {
this.renderTaskAbort = undefined;
}
} catch (e) {
showPreloadErrors(this.element, e);
}
}

Expand Down Expand Up @@ -2188,14 +2221,14 @@ async function webviewPreloads(ctx: PreloadContext) {

public rerender() {
if (this._content) {
this.render(this._content.content, this._content.preloadsAndErrors);
this.render(this._content.content, this._content.rendererId, this._content.preloadsAndErrors);
}
}

public updateAndRerender(content: webviewMessages.ICreationContent) {
if (this._content) {
this._content.content = content;
this.render(this._content.content, this._content.preloadsAndErrors);
this.render(this._content.content, this._content.rendererId, this._content.preloadsAndErrors);
}
}
}
Expand Down

0 comments on commit 3f92eb6

Please sign in to comment.