Skip to content

Commit

Permalink
Use text encoder decoder and not buffer (#15102)
Browse files Browse the repository at this point in the history 
* Use TextEncoder/TextDecoder and not Buffer

* oops

* Fix tests
  • Loading branch information
@DonJayamanne
DonJayamanne authored Feb 1, 2024
1 parent 704d252 commit 606b065
Show file tree
Hide file tree
Showing 20 changed files with 173 additions and 184 deletions.
4 changes: 2 additions & 2 deletions src/kernels/execution/cellExecutionMessageHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ export class CellExecutionMessageHandler implements IDisposable {
return true;
}
if (mime === WIDGET_MIMETYPE) {
const data: WidgetData = JSON.parse(Buffer.from(outputItem.data).toString());
const data: WidgetData = JSON.parse(new TextDecoder().decode(outputItem.data));
// Jupyter Output widgets cannot be rendered properly by the widget manager,
// We need to render that.
if (typeof data.model_id === 'string' && this.commIdsMappedToWidgetOutputModels.has(data.model_id)) {
Expand Down Expand Up @@ -763,7 +763,7 @@ export class CellExecutionMessageHandler implements IDisposable {
return false;
}
try {
const value = JSON.parse(Buffer.from(outputItem.data).toString()) as { model_id?: string };
const value = JSON.parse(new TextDecoder().decode(outputItem.data)) as { model_id?: string };
return value.model_id === expectedModelId;
} catch (ex) {
traceWarning(`Failed to deserialize the widget data`, ex);
Expand Down
128 changes: 100 additions & 28 deletions src/kernels/execution/cellExecutionMessageHandler.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
// Update the display data again.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayDataWorld, 1, (producer) => {
return [
Expand All @@ -429,7 +432,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'World'
);
});

test('Execute cell and add Display output (even if Cell DOM has not yet been updated) ', async () => {
Expand Down Expand Up @@ -471,7 +477,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
// Update the display data again.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayDataWorld, 1, (producer) => {
return [
Expand All @@ -482,7 +491,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'World'
);
});
test('Updates to two separate display updates in the same cell output', async () => {
const notebook = createNotebook([
Expand Down Expand Up @@ -520,8 +532,14 @@ suite(`Cell Execution Message Handler`, () => {
assert.strictEqual((output1.transient as any).display_id, display_id);
const output2 = translateCellDisplayOutput(notebook.cellAt(0).outputs[1]);
assert.strictEqual((output2.transient as any).display_id, display_id2);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayData1ILike, 2, (producer) => {
Expand All @@ -534,8 +552,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1ILike, 2, outputsFromILikeUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the second display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1Pizza, 3, (producer) => {
Expand All @@ -548,8 +572,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1Pizza, 3, outputsFromPizzaUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates to two separate display updates in the same cell output (update second display update)', async () => {
Expand Down Expand Up @@ -589,8 +619,14 @@ suite(`Cell Execution Message Handler`, () => {
assert.strictEqual((output1.transient as any).display_id, display_id);
const output2 = translateCellDisplayOutput(notebook.cellAt(0).outputs[1]);
assert.strictEqual((output2.transient as any).display_id, display_id2);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the second display data.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayData1Pizza, 2, (producer) => {
Expand All @@ -604,8 +640,14 @@ suite(`Cell Execution Message Handler`, () => {
});

// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1Pizza, 2, outputsFromPizzaUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1ILike, 3, (producer) => {
Expand All @@ -618,8 +660,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1ILike, 3, outputsFromILikeUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates to two separate display updates in the same cell output (even if Cell DOM has not yet been updated)', async () => {
Expand Down Expand Up @@ -667,8 +715,14 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1ILike, 3, (producer) => {
Expand All @@ -680,8 +734,14 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates display updates in the same cell output within the same execution (even if Cell DOM has not yet been updated) (Issue 12755, 13105, 13163)', async () => {
Expand Down Expand Up @@ -741,11 +801,23 @@ suite(`Cell Execution Message Handler`, () => {
producer.stream({ name: 'stdout', text: 'Pizza\n' })
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Touch me not\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'C');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[0].data).toString(), 'Hello\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[1].data).toString(), 'World\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[2].data).toString(), 'Pizza\n');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Touch me not\n'
);
assert.strictEqual(new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'C');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[0].data).toString(),
'Hello\n'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[1].data).toString(),
'World\n'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[2].data).toString(),
'Pizza\n'
);
});
});

Expand Down Expand Up @@ -792,7 +864,7 @@ suite(`Cell Execution Message Handler`, () => {
for (let index = 0; index < cell.outputs[0].items.length; index++) {
const item = cell.outputs[0].items[index];
assert.strictEqual(item.mime, 'application/vnd.code.notebook.stdout');
assert.strictEqual(Buffer.from(item.data).toString(), `${index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${index}\n`);
}

// Now assume we closed VS Code, and then opened it again.
Expand Down Expand Up @@ -828,9 +900,9 @@ suite(`Cell Execution Message Handler`, () => {
const item = cell.outputs[0].items[index];
assert.strictEqual(item.mime, 'application/vnd.code.notebook.stdout');
if (index >= 50) {
assert.strictEqual(Buffer.from(item.data).toString(), `${25 + index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${25 + index}\n`);
} else {
assert.strictEqual(Buffer.from(item.data).toString(), `${index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${index}\n`);
}
}
}
Expand Down
24 changes: 5 additions & 19 deletions src/kernels/execution/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { StopWatch } from '../../platform/common/utils/stopWatch';
import { getExtensionSpecifcStack } from '../../platform/errors/errors';
import { getVersion } from '../../platform/interpreter/helpers';
import { base64ToUint8Array, uint8ArrayToBase64 } from '../../platform/common/utils/string';

export enum CellOutputMimeTypes {
error = 'application/vnd.code.notebook.error',
Expand Down Expand Up @@ -112,7 +113,7 @@ const orderOfMimeTypes = [
function isEmptyVendoredMimeType(outputItem: NotebookCellOutputItem) {
if (outputItem.mime.startsWith('application/vnd.')) {
try {
return Buffer.from(outputItem.data).toString().length === 0;
return new TextDecoder().decode(outputItem.data).length === 0;
} catch {}
}
return false;
Expand Down Expand Up @@ -372,7 +373,7 @@ export function translateCellErrorOutput(output: NotebookCellOutput): nbformat.I
};
}
const originalError: undefined | nbformat.IError = output.metadata?.originalError;
const value: Error = JSON.parse(Buffer.from(firstItem.data as Uint8Array).toString('utf8'));
const value: Error = JSON.parse(new TextDecoder().decode(firstItem.data));
return {
output_type: 'error',
ename: value.name,
Expand Down Expand Up @@ -400,17 +401,7 @@ function convertOutputMimeToJupyterOutput(mime: string, value: Uint8Array) {
} else if (mime.startsWith('image/') && mime !== 'image/svg+xml') {
// Images in Jupyter are stored in base64 encoded format.
// VS Code expects bytes when rendering images.
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return Buffer.from(value).toString('base64');
} else {
// https://developer.mozilla.org/en-US/docs/Glossary/Base64#solution_1_%E2%80%93_escaping_the_string_before_encoding_it
const stringValue = textDecoder.decode(value);
return btoa(
encodeURIComponent(stringValue).replace(/%([0-9A-F]{2})/g, function (_match, p1) {
return String.fromCharCode(Number.parseInt('0x' + p1));
})
);
}
return uint8ArrayToBase64(value);
} else if (
mime.toLowerCase().startsWith('application/vnd.holoviews_load.v') &&
mime.toLowerCase().endsWith('+json')
Expand Down Expand Up @@ -450,12 +441,7 @@ function convertJupyterOutputToBuffer(mime: string, value: unknown): NotebookCel
} else if (mime.startsWith('image/') && typeof value === 'string' && mime !== 'image/svg+xml') {
// Images in Jupyter are stored in base64 encoded format.
// VS Code expects bytes when rendering images.
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return new NotebookCellOutputItem(Buffer.from(value, 'base64'), mime);
} else {
const data = Uint8Array.from(atob(value), (c) => c.charCodeAt(0));
return new NotebookCellOutputItem(data, mime);
}
return new NotebookCellOutputItem(base64ToUint8Array(value), mime);
} else if (typeof value === 'object' && value !== null && !Array.isArray(value)) {
return NotebookCellOutputItem.text(JSON.stringify(value), mime);
} else {
Expand Down
12 changes: 6 additions & 6 deletions src/kernels/kernelCrashMonitor.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ suite('Kernel Crash Monitor', () => {
const execution = controller.createNotebookCellExecution(cell);
execution.start();

const expectedErrorMessage = Buffer.from(
const expectedErrorMessage = new TextDecoder().decode(
createOutputWithErrorMessageForDisplay(DataScience.kernelCrashedDueToCodeInCurrentOrPreviousCell)?.items[0]!
.data!
).toString();
);

when(kernel.status).thenReturn('dead');
onKernelStatusChanged.fire({ status: 'dead', kernel: instance(kernel) });
Expand All @@ -127,7 +127,7 @@ suite('Kernel Crash Monitor', () => {
assert.strictEqual(cell.outputs.length, 1);
assert.strictEqual(cell.outputs[0].items.length, 1);
const outputItem = cell.outputs[0].items[0];
assert.include(Buffer.from(outputItem.data).toString(), expectedErrorMessage);
assert.include(new TextDecoder().decode(outputItem.data), expectedErrorMessage);
});
test('Error message displayed and Cell output updated with error message (jupyter kernel)', async () => {
when(kernelSession.kind).thenReturn('localJupyter');
Expand All @@ -139,10 +139,10 @@ suite('Kernel Crash Monitor', () => {
const execution = controller.createNotebookCellExecution(cell);
execution.start();

const expectedErrorMessage = Buffer.from(
const expectedErrorMessage = new TextDecoder().decode(
createOutputWithErrorMessageForDisplay(DataScience.kernelCrashedDueToCodeInCurrentOrPreviousCell)?.items[0]!
.data!
).toString();
);

when(kernel.status).thenReturn('autorestarting');
when(kernelSession.status).thenReturn('autorestarting');
Expand All @@ -160,6 +160,6 @@ suite('Kernel Crash Monitor', () => {
assert.strictEqual(cell.outputs.length, 1);
assert.strictEqual(cell.outputs[0].items.length, 1);
const outputItem = cell.outputs[0].items[0];
assert.include(Buffer.from(outputItem.data).toString(), expectedErrorMessage);
assert.include(new TextDecoder().decode(outputItem.data), expectedErrorMessage);
});
});
2 changes: 1 addition & 1 deletion src/notebooks/export/exportBase.web.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class ExportBase implements IExportBase {
});
const bytes = this.b64toBlob(content.content, 'application/pdf');
const buffer = await bytes.arrayBuffer();
await this.fs.writeFile(target!, Buffer.from(buffer));
await this.fs.writeFile(target!, new Uint8Array(buffer));
} else {
const content = await contentsManager.get(tempTarget, {
type: 'file',
Expand Down
Loading

0 comments on commit 606b065

Please sign in to comment.