Skip to content

Commit

Permalink
Retain original sandbox errors (from different JavaScript realms) wit…
Browse files Browse the repository at this point in the history
…hout coercion (#355)

This PR retains original errors being returned from different realms. Currently, this module [wraps any such errors](https://github.com/guigrpa/docx-templates/blob/master/src/jsSandbox.ts#L54) as `new Error(`${err}`)`, coercing their name/message into a string, losing these as separate fields, and losing the stack trace entirely (it gets effectively replaced when the error is re-thrown). The reason it does this is because `instanceof Error` returns false for sandboxed errors when the prototype chain doesn't match; these errors are still Error objects, just that they can't be detected with `instanceof`.

This change affects the formatting of `CommandExeuctionError`, since the string coercion no longer happens automatically, so the `${err.name}` has been added back in to its message. Most tests continue to work unchanged, but several have required updating now that this messaging is consistent between sandboxing and unsandboxed execution.
  • Loading branch information
davidjb authored Mar 21, 2024
1 parent ac2e4ae commit 885af4d
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 13 deletions.
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)
);
}

/**
* 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

0 comments on commit 885af4d

Please sign in to comment.