Skip to content
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

Retain original sandbox errors (from different JavaScript realms) without coercion #355

Merged
merged 1 commit into from
Mar 21, 2024
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
4 changes: 2 additions & 2 deletions src/__tests__/__snapshots__/error_handling.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ exports[`sandbox custom ErrorHandler properly handles InvalidCommandError 1`] =
</w:p>
<w:p w:rsidR="00537D95" w:rsidRPr="0070400C" w:rsidRDefault="00537D95">
<w:r>
<w:t xml:space="preserve">Error: SyntaxError: Unexpected identifier 'foo'</w:t>
<w:t xml:space="preserve">SyntaxError: Unexpected identifier 'foo'</w:t>
</w:r>
</w:p>
<w:p w:rsidR="00413949" w:rsidRPr="0070400C" w:rsidRDefault="00413949">
Expand All @@ -284,7 +284,7 @@ exports[`sandbox custom ErrorHandler properly handles InvalidCommandError 1`] =

exports[`sandbox custom ErrorHandler properly handles InvalidCommandError 2`] = `
[
[Error: SyntaxError: Unexpected identifier 'foo'],
[SyntaxError: Unexpected identifier 'foo'],
]
`;

Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/__snapshots__/images.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ exports[`001: Issue #61 Correctly renders an SVG image 1`] = `
}
`;

exports[`002: throws when thumbnail is incorrectly provided when inserting an SVG 1`] = `[Error: Error executing command 'IMAGE svgImgFile()': An extension (one of .png,.gif,.jpg,.jpeg,.svg) needs to be provided when providing an image or a thumbnail.]`;
exports[`002: throws when thumbnail is incorrectly provided when inserting an SVG 1`] = `[Error: Error executing command 'IMAGE svgImgFile()': Error: An extension (one of .png,.gif,.jpg,.jpeg,.svg) needs to be provided when providing an image or a thumbnail.]`;

exports[`003: can inject an svg without a thumbnail 1`] = `
{
Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/__snapshots__/templating.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19868,7 +19868,7 @@ exports[`noSandbox Template processing 39 Processes LINK commands 1`] = `
}
`;

exports[`noSandbox Template processing 40 Throws on invalid command 1`] = `[Error: Error executing command 'TTT foo': Unexpected identifier 'foo']`;
exports[`noSandbox Template processing 40 Throws on invalid command 1`] = `[Error: Error executing command 'TTT foo': SyntaxError: Unexpected identifier 'foo']`;

exports[`noSandbox Template processing 41 Throws on invalid for logic 1`] = `[Error: Invalid command: END-FOR company]`;

Expand Down Expand Up @@ -26172,14 +26172,14 @@ exports[`noSandbox Template processing 107b non-alphanumeric INS commands (e.g.

exports[`noSandbox Template processing 112a failFast: false lists all errors in the document before failing. 1`] = `
[
[Error: Error executing command 'notavailable': notavailable is not defined],
[Error: Error executing command 'something': something is not defined],
[Error: Error executing command 'notavailable': ReferenceError: notavailable is not defined],
[Error: Error executing command 'something': ReferenceError: something is not defined],
[Error: Invalid command: END-FOR company],
[Error: Unterminated FOR-loop ('FOR company'). Make sure each FOR loop has a corresponding END-FOR command.],
]
`;

exports[`noSandbox Template processing 112b failFast: true has the same behaviour as when failFast is undefined 1`] = `[Error: Error executing command 'notavailable': notavailable is not defined]`;
exports[`noSandbox Template processing 112b failFast: true has the same behaviour as when failFast is undefined 1`] = `[Error: Error executing command 'notavailable': ReferenceError: notavailable is not defined]`;

exports[`noSandbox Template processing 131 correctly handles Office 365 .docx files 1`] = `
{
Expand Down Expand Up @@ -30290,7 +30290,7 @@ exports[`noSandbox Template processing avoids confusion between variable name an
}
`;

exports[`noSandbox Template processing fixSmartQuotes flag (see PR #152) 1`] = `"Error executing command 'reverse(‘aubergine’)': Invalid or unexpected token"`;
exports[`noSandbox Template processing fixSmartQuotes flag (see PR #152) 1`] = `"Error executing command 'reverse(‘aubergine’)': SyntaxError: Invalid or unexpected token"`;

exports[`noSandbox Template processing iterate over object properties and keys in FOR loop 1`] = `
{
Expand Down
73 changes: 73 additions & 0 deletions src/__tests__/error_handling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@ import QR from 'qrcode';
import { createReport } from '../index';
import { setDebugLogSink } from '../debug';
import {
isError,
NullishCommandResultError,
CommandExecutionError,
InvalidCommandError,
} from '../errors';

if (process.env.DEBUG) setDebugLogSink(console.log);

class NoErrorThrownError extends Error {}

const getError = async <TError>(call: () => unknown): Promise<TError> => {
try {
await call();
throw new NoErrorThrownError();
} catch (error: unknown) {
return error as TError;
}
};

['noSandbox', 'sandbox'].forEach(sbStatus => {
const noSandbox = sbStatus === 'sandbox' ? false : true;

Expand Down Expand Up @@ -325,3 +337,64 @@ if (process.env.DEBUG) setDebugLogSink(console.log);
);
});
});

describe('errors from different realms', () => {
it('sandbox', async () => {
const template = await fs.promises.readFile(
path.join(__dirname, 'fixtures', 'referenceError.docx')
);

const error = await getError(() =>
createReport({ noSandbox: false, template, data: {} })
);
expect(error).toBeInstanceOf(CommandExecutionError);

// We cannot check with instanceof as this Error is from another realm despite still being an error
const commandExecutionError = error as CommandExecutionError;
expect(commandExecutionError.err).not.toBeInstanceOf(ReferenceError);
expect(commandExecutionError.err).not.toBeInstanceOf(Error);
expect(commandExecutionError.err.name).toBe('ReferenceError');
expect(commandExecutionError.err.message).toBe(
'nonExistentVariable is not defined'
);
});

it('noSandbox', async () => {
const template = await fs.promises.readFile(
path.join(__dirname, 'fixtures', 'referenceError.docx')
);

const error = await getError(() =>
createReport({ noSandbox: true, template, data: {} })
);
expect(error).toBeInstanceOf(CommandExecutionError);

// Without sandboxing, the error is from the same realm
const commandExecutionError = error as CommandExecutionError;
expect(commandExecutionError.err).toBeInstanceOf(ReferenceError);
expect(commandExecutionError.err).toBeInstanceOf(Error);
expect(commandExecutionError.err.name).toBe('ReferenceError');
expect(commandExecutionError.err.message).toBe(
'nonExistentVariable is not defined'
);
});
});

describe('isError', () => {
it('Error is an error', () => {
expect(isError(new Error())).toBeTruthy();
});

it('error-like object is an error', () => {
expect(
isError({
name: 'ReferenceError',
message: 'nonExistentVariable is not defined',
})
).toBeTruthy();
});

it('primitive is not an error', () => {
expect(isError(1)).toBeFalsy();
});
});
Binary file added src/__tests__/fixtures/referenceError.docx
Binary file not shown.
9 changes: 8 additions & 1 deletion src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { LoopStatus } from './types';

export function isError(err: unknown): err is Error {
return (
err instanceof Error ||
(typeof err === 'object' && !!err && 'name' in err && 'message' in err)
jjhbw marked this conversation as resolved.
Show resolved Hide resolved
);
}

/**
* Thrown when `rejectNullish` is set to `true` and a command returns `null` or `undefined`.
*/
Expand Down Expand Up @@ -50,7 +57,7 @@ export class CommandExecutionError extends Error {
command: string;
err: Error;
constructor(err: Error, command: string) {
super(`Error executing command '${command}': ${err.message}`);
super(`Error executing command '${command}': ${err.name}: ${err.message}`);
Object.setPrototypeOf(this, CommandExecutionError.prototype);
this.command = command;
this.err = err;
Expand Down
8 changes: 6 additions & 2 deletions src/jsSandbox.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import vm from 'vm';
import { getCurLoop } from './reportUtils';
import { ReportData, Context, SandBox } from './types';
import { CommandExecutionError, NullishCommandResultError } from './errors';
import {
isError,
CommandExecutionError,
NullishCommandResultError,
} from './errors';
import { logger } from './debug';

// Runs a user snippet in a sandbox, and returns the result.
Expand Down Expand Up @@ -51,7 +55,7 @@ export async function runUserJsAndGetRaw(
result = await script.runInContext(context);
}
} catch (err) {
const e = err instanceof Error ? err : new Error(`${err}`);
const e = isError(err) ? err : new Error(`${err}`);
if (ctx.options.errorHandler != null) {
context = sandbox;
result = await ctx.options.errorHandler(e, code);
Expand Down
5 changes: 3 additions & 2 deletions src/processTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
NonTextNode,
} from './types';
import {
isError,
CommandSyntaxError,
InternalError,
InvalidCommandError,
Expand Down Expand Up @@ -628,7 +629,7 @@ const processCmd: CommandProcessor = async (
try {
processImage(ctx, img);
} catch (e) {
if (!(e instanceof Error)) throw e;
if (!isError(e)) throw e;
throw new ImageError(e, cmd);
}
}
Expand Down Expand Up @@ -660,7 +661,7 @@ const processCmd: CommandProcessor = async (
} else throw new CommandSyntaxError(cmd);
return;
} catch (err) {
if (!(err instanceof Error)) throw err;
if (!isError(err)) throw err;
if (ctx.options.errorHandler != null) {
return ctx.options.errorHandler(err);
}
Expand Down
Loading