Skip to content

Commit

Permalink
fix(wasm): rejecting with error on node.js file read
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nickbabcock committed Nov 28, 2022
1 parent 570f77c commit 7117524
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 25 deletions.
4 changes: 2 additions & 2 deletions packages/wasm/src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ return new Promise((resolve, reject) => {
fs.readFile(path.resolve(__dirname, filepath), (error, buffer) => {
if (error != null) {
reject(error)
} else {
resolve(_instantiateOrCompile(buffer, imports, false))
}
resolve(_instantiateOrCompile(buffer, imports, false))
});
});
`;
Expand Down
64 changes: 41 additions & 23 deletions packages/wasm/test/test.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createRequire } from 'module';
import { sep, posix, join } from 'path';
import { fileURLToPath } from 'url';
import { Worker } from 'worker_threads';

import { rollup } from 'rollup';
import globby from 'globby';
Expand Down Expand Up @@ -96,32 +97,26 @@ test('imports', async (t) => {
await testBundle(t, bundle);
});

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

const bundle = await rollup({
input: 'fixtures/worker.js',
plugins: [wasmPlugin()]
});
const code = await getCode(bundle);
const executeWorker = () => {
const worker = new Worker(code, { eval: true });
return new Promise((resolve, reject) => {
worker.on('error', (err) => reject(err));
worker.on('exit', (exitCode) => resolve(exitCode));
});
};
await t.notThrowsAsync(async () => {
const result = await executeWorker();
t.true(result === 0);
const bundle = await rollup({
input: 'fixtures/worker.js',
plugins: [wasmPlugin()]
});
const code = await getCode(bundle);
const executeWorker = () => {
const worker = new Worker(code, { eval: true });
return new Promise((resolve, reject) => {
worker.on('error', (err) => reject(err));
worker.on('exit', (exitCode) => resolve(exitCode));
});
};
await t.notThrowsAsync(async () => {
const result = await executeWorker();
t.true(result === 0);
});
} catch (err) {
// worker threads aren't fully supported in Node versions before 11.7.0
}
});

test('injectHelper', async (t) => {
t.plan(4);
Expand Down Expand Up @@ -216,3 +211,26 @@ test('works as CJS plugin', async (t) => {
});
await testBundle(t, bundle);
});

test('avoid uncaught exception on file read', async (t) => {
t.plan(2);

const bundle = await rollup({
input: 'fixtures/complex.js',
plugins: [wasmPlugin({ maxFileSize: 0, targetEnv: 'node' })]
});

const raw = await getCode(bundle);
const code = raw.replace('.wasm', '-does-not-exist.wasm');

const executeWorker = () => {
const worker = new Worker(`let result; ${code}`, { eval: true });
return new Promise((resolve, reject) => {
worker.on('error', (err) => reject(err));
worker.on('exit', (exitCode) => resolve(exitCode));
});
};

const err = await t.throwsAsync(() => executeWorker());
t.regex(err.message, /no such file or directory/);
});

0 comments on commit 7117524

Please sign in to comment.