Skip to content

Commit

Permalink
worker: allow URL in Worker constructor
Browse files Browse the repository at this point in the history
The explicit goal is to let users use `import.meta.url` to
re-load thecurrent module inside a Worker instance.

Fixes: #30780
PR-URL: #31664
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
aduh95 authored and MylesBorins committed Mar 24, 2020
1 parent 597bcb5 commit e83dcde
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 20 deletions.
16 changes: 11 additions & 5 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ if (isMainThread) {
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/31664
description: The `filename` parameter can be a WHATWG `URL` object using
`file:` protocol.
- version: v13.2.0
pr-url: https://github.com/nodejs/node/pull/26628
description: The `resourceLimits` option was introduced.
Expand All @@ -521,9 +525,10 @@ changes:
description: The `argv` option was introduced.
-->

* `filename` {string} The path to the Worker’s main script. Must be
either an absolute path or a relative path (i.e. relative to the
current working directory) starting with `./` or `../`.
* `filename` {string|URL} The path to the Worker’s main script or module. Must
be either an absolute path or a relative path (i.e. relative to the
current working directory) starting with `./` or `../`, or a WHATWG `URL`
object using `file:` protocol.
If `options.eval` is `true`, this is a string containing JavaScript code
rather than a path.
* `options` {Object}
Expand All @@ -536,8 +541,9 @@ changes:
to specify that the parent thread and the child thread should share their
environment variables; in that case, changes to one thread’s `process.env`
object will affect the other thread as well. **Default:** `process.env`.
* `eval` {boolean} If `true`, interpret the first argument to the constructor
as a script that is executed once the worker is online.
* `eval` {boolean} If `true` and the first argument is a `string`, interpret
the first argument to the constructor as a script that is executed once the
worker is online.
* `execArgv` {string[]} List of node CLI options passed to the worker.
V8 options (such as `--max-old-space-size`) and options that affect the
process (such as `--title`) are not supported. If set, this will be provided
Expand Down
11 changes: 8 additions & 3 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1391,9 +1391,14 @@ E('ERR_WORKER_INVALID_EXEC_ARGV', (errors, msg = 'invalid execArgv flags') =>
E('ERR_WORKER_NOT_RUNNING', 'Worker instance not running', Error);
E('ERR_WORKER_OUT_OF_MEMORY',
'Worker terminated due to reaching memory limit: %s', Error);
E('ERR_WORKER_PATH',
'The worker script filename must be an absolute path or a relative ' +
'path starting with \'./\' or \'../\'. Received "%s"',
E('ERR_WORKER_PATH', (filename) =>
'The worker script or module filename must be an absolute path or a ' +
'relative path starting with \'./\' or \'../\'.' +
(filename.startsWith('file://') ?
' If you want to pass a file:// URL, you must wrap it around `new URL`.' :
''
) +
` Received "${filename}"`,
TypeError);
E('ERR_WORKER_UNSERIALIZABLE_ERROR',
'Serializing an uncaught exception failed', Error);
Expand Down
12 changes: 8 additions & 4 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -1348,8 +1348,7 @@ function getPathFromURLPosix(url) {
function fileURLToPath(path) {
if (typeof path === 'string')
path = new URL(path);
else if (path == null || !path[searchParams] ||
!path[searchParams][searchParams])
else if (!isURLInstance(path))
throw new ERR_INVALID_ARG_TYPE('path', ['string', 'URL'], path);
if (path.protocol !== 'file:')
throw new ERR_INVALID_URL_SCHEME('file');
Expand Down Expand Up @@ -1396,9 +1395,13 @@ function pathToFileURL(filepath) {
return outURL;
}

function isURLInstance(fileURLOrPath) {
return fileURLOrPath != null && fileURLOrPath[searchParams] &&
fileURLOrPath[searchParams][searchParams];
}

function toPathIfFileURL(fileURLOrPath) {
if (fileURLOrPath == null || !fileURLOrPath[searchParams] ||
!fileURLOrPath[searchParams][searchParams])
if (!isURLInstance(fileURLOrPath))
return fileURLOrPath;
return fileURLToPath(fileURLOrPath);
}
Expand Down Expand Up @@ -1431,6 +1434,7 @@ module.exports = {
fileURLToPath,
pathToFileURL,
toPathIfFileURL,
isURLInstance,
URL,
URLSearchParams,
domainToASCII,
Expand Down
34 changes: 27 additions & 7 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const {
ERR_INVALID_ARG_TYPE,
// eslint-disable-next-line no-unused-vars
ERR_WORKER_INIT_FAILED,
ERR_INVALID_ARG_VALUE,
} = errorCodes;
const { validateString } = require('internal/validators');
const { getOptionValue } = require('internal/options');

const workerIo = require('internal/worker/io');
Expand All @@ -45,7 +45,7 @@ const {
WritableWorkerStdio
} = workerIo;
const { deserializeError } = require('internal/error-serdes');
const { pathToFileURL } = require('url');
const { fileURLToPath, isURLInstance, pathToFileURL } = require('internal/url');

const {
ownsProcessState,
Expand Down Expand Up @@ -86,7 +86,6 @@ class Worker extends EventEmitter {
constructor(filename, options = {}) {
super();
debug(`[${threadId}] create new worker`, filename, options);
validateString(filename, 'filename');
if (options.execArgv && !ArrayIsArray(options.execArgv)) {
throw new ERR_INVALID_ARG_TYPE('options.execArgv',
'Array',
Expand All @@ -99,11 +98,33 @@ class Worker extends EventEmitter {
}
argv = options.argv.map(String);
}
if (!options.eval) {
if (!path.isAbsolute(filename) && !/^\.\.?[\\/]/.test(filename)) {

let url;
if (options.eval) {
if (typeof filename !== 'string') {
throw new ERR_INVALID_ARG_VALUE(
'options.eval',
options.eval,
'must be false when \'filename\' is not a string'
);
}
url = null;
} else {
if (isURLInstance(filename)) {
url = filename;
filename = fileURLToPath(filename);
} else if (typeof filename !== 'string') {
throw new ERR_INVALID_ARG_TYPE(
'filename',
['string', 'URL'],
filename
);
} else if (path.isAbsolute(filename) || /^\.\.?[\\/]/.test(filename)) {
filename = path.resolve(filename);
url = pathToFileURL(filename);
} else {
throw new ERR_WORKER_PATH(filename);
}
filename = path.resolve(filename);

const ext = path.extname(filename);
if (ext !== '.js' && ext !== '.mjs' && ext !== '.cjs') {
Expand All @@ -125,7 +146,6 @@ class Worker extends EventEmitter {
options.env);
}

const url = options.eval ? null : pathToFileURL(filename);
// Set up the C++ handle for the worker, as well as some internal wiring.
this[kHandle] = new WorkerImpl(url,
env === process.env ? null : env,
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-worker-type-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ const { Worker } = require('worker_threads');
{
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "filename" argument must be of type string.' +
message: 'The "filename" argument must be of type string ' +
'or an instance of URL.' +
common.invalidArgTypeHelper(val)
}
);
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-worker-unsupported-eval-on-url.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import '../common/index.mjs';
import assert from 'assert';
import { Worker } from 'worker_threads';

const re = /The argument 'options\.eval' must be false when 'filename' is not a string\./;
assert.throws(() => new Worker(new URL(import.meta.url), { eval: true }), re);
24 changes: 24 additions & 0 deletions test/parallel/test-worker-unsupported-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const { Worker } = require('worker_threads');
assert.throws(() => { new Worker('/b'); }, expectedErr);
assert.throws(() => { new Worker('/c.wasm'); }, expectedErr);
assert.throws(() => { new Worker('/d.txt'); }, expectedErr);
assert.throws(() => { new Worker(new URL('file:///C:/e.wasm')); }, expectedErr);
}

{
Expand All @@ -26,3 +27,26 @@ const { Worker } = require('worker_threads');
assert.throws(() => { new Worker('file:///file_url'); }, expectedErr);
assert.throws(() => { new Worker('https://www.url.com'); }, expectedErr);
}

{
assert.throws(
() => { new Worker('file:///file_url'); },
/If you want to pass a file:\/\/ URL, you must wrap it around `new URL`/
);
assert.throws(
() => { new Worker('relative_no_dot'); },
// eslint-disable-next-line node-core/no-unescaped-regexp-dot
/^((?!If you want to pass a file:\/\/ URL, you must wrap it around `new URL`).)*$/s
);
}

{
const expectedErr = {
code: 'ERR_INVALID_URL_SCHEME',
name: 'TypeError'
};
assert.throws(() => { new Worker(new URL('https://www.url.com')); },
expectedErr);
assert.throws(() => { new Worker(new URL('data:application/javascript,')); },
expectedErr);
}
18 changes: 18 additions & 0 deletions test/parallel/test-worker.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { mustCall } from '../common/index.mjs';
import assert from 'assert';
import { Worker, isMainThread, parentPort } from 'worker_threads';

const TEST_STRING = 'Hello, world!';

if (isMainThread) {
const w = new Worker(new URL(import.meta.url));
w.on('message', mustCall((message) => {
assert.strictEqual(message, TEST_STRING);
}));
} else {
setImmediate(() => {
process.nextTick(() => {
parentPort.postMessage(TEST_STRING);
});
});
}

0 comments on commit e83dcde

Please sign in to comment.