Skip to content

Commit 5af80b2

Browse files
authored
fix(wasm): rejecting with error on node.js file read (#1346)
When node.js is reading the wasm file and encounters an error (like if the file doesn't exist or user doesn't have permissions to read). It will still try and call `_instantiateOrCompile` with `undefined`, which results in an uncaught exception, possibly obscuring the error. As instead of the file read error, the following error will return: > TypeError: WebAssembly.compile(): Argument 0 must be a buffer source This can be replicated with the following script (don't execute at a REPL): ```js (async () => { await new Promise((resolve, reject) => { reject(new Error("permission error")); resolve(WebAssembly.compile(undefined)); }); })(); ``` Node.js will exit with an error code and print only the type error to stderr. If we try and catch the error at the outer level ```js (async () => { try { await new Promise((resolve, reject) => { reject(new Error("permission error")); resolve(WebAssembly.compile(undefined)); }); } catch (ex) { console.log(`this is the error: ${ex}`) } })(); ``` The log statement will exclude the type error: > this is the error: Error: permission error But node.js will still exit with an error code and the type error is printed to stderr. This is the behavior of node.js default [uncaught exception handler](https://nodejs.org/api/process.html#event-uncaughtexception) > By default, Node.js handles such exceptions by printing the stack trace to stderr and exiting with code 1 This commit avoids the uncaught exception by only invoking `resolve` behind a conditional on an absent error. This commit doesn't change the status of the promise as calling `resolve` after `reject`, while legal, doesn't really do anything [(ref)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/Promise#description): > Only the first call to `resolveFunc` or `rejectFunc` affects the promise's > eventual state, and subsequent calls to either function can neither change the > fulfillment value/rejection reason nor toggle its eventual state from > "fulfilled" to "rejected" or opposite So depending on the runtime and how the user is overriding the default handler, uncaught exceptions may be handled differently. The test executes in a worker so that it can require the appropiate node js modules. I saw that another Wasm test used a worker, but would never execute as require is not defined, so this commit also fixes that test case.
1 parent 650f21a commit 5af80b2

File tree

2 files changed

+46
-25
lines changed

2 files changed

+46
-25
lines changed

packages/wasm/src/helper.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ return new Promise((resolve, reject) => {
1010
fs.readFile(path.resolve(__dirname, filepath), (error, buffer) => {
1111
if (error != null) {
1212
reject(error)
13+
} else {
14+
resolve(_instantiateOrCompile(buffer, imports, false))
1315
}
14-
15-
resolve(_instantiateOrCompile(buffer, imports, false))
1616
});
1717
});
1818
`;

packages/wasm/test/test.mjs

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createRequire } from 'module';
22
import { sep, posix, join } from 'path';
33
import { fileURLToPath } from 'url';
4+
import { Worker } from 'worker_threads';
45

56
import { rollup } from 'rollup';
67
import globby from 'globby';
@@ -96,32 +97,26 @@ test('imports', async (t) => {
9697
await testBundle(t, bundle);
9798
});
9899

99-
try {
100-
// eslint-disable-next-line global-require
101-
const { Worker } = require('worker_threads');
102-
test('worker', async (t) => {
103-
t.plan(2);
100+
test('worker', async (t) => {
101+
t.plan(2);
104102

105-
const bundle = await rollup({
106-
input: 'fixtures/worker.js',
107-
plugins: [wasmPlugin()]
108-
});
109-
const code = await getCode(bundle);
110-
const executeWorker = () => {
111-
const worker = new Worker(code, { eval: true });
112-
return new Promise((resolve, reject) => {
113-
worker.on('error', (err) => reject(err));
114-
worker.on('exit', (exitCode) => resolve(exitCode));
115-
});
116-
};
117-
await t.notThrowsAsync(async () => {
118-
const result = await executeWorker();
119-
t.true(result === 0);
103+
const bundle = await rollup({
104+
input: 'fixtures/worker.js',
105+
plugins: [wasmPlugin()]
106+
});
107+
const code = await getCode(bundle);
108+
const executeWorker = () => {
109+
const worker = new Worker(code, { eval: true });
110+
return new Promise((resolve, reject) => {
111+
worker.on('error', (err) => reject(err));
112+
worker.on('exit', (exitCode) => resolve(exitCode));
120113
});
114+
};
115+
await t.notThrowsAsync(async () => {
116+
const result = await executeWorker();
117+
t.true(result === 0);
121118
});
122-
} catch (err) {
123-
// worker threads aren't fully supported in Node versions before 11.7.0
124-
}
119+
});
125120

126121
test('injectHelper', async (t) => {
127122
t.plan(4);
@@ -233,3 +228,29 @@ test('works as CJS plugin', async (t) => {
233228
});
234229
await testBundle(t, bundle);
235230
});
231+
232+
// uncaught exception will cause test failures on this node version.
233+
if (!process.version.startsWith('v14')) {
234+
test('avoid uncaught exception on file read', async (t) => {
235+
t.plan(2);
236+
237+
const bundle = await rollup({
238+
input: 'fixtures/complex.js',
239+
plugins: [wasmPlugin({ maxFileSize: 0, targetEnv: 'node' })]
240+
});
241+
242+
const raw = await getCode(bundle);
243+
const code = raw.replace('.wasm', '-does-not-exist.wasm');
244+
245+
const executeWorker = () => {
246+
const worker = new Worker(`let result; ${code}`, { eval: true });
247+
return new Promise((resolve, reject) => {
248+
worker.on('error', (err) => reject(err));
249+
worker.on('exit', (exitCode) => resolve(exitCode));
250+
});
251+
};
252+
253+
const err = await t.throwsAsync(() => executeWorker());
254+
t.regex(err.message, /no such file or directory/);
255+
});
256+
}

0 commit comments

Comments
 (0)