andywer/threads-plugin

electron - bundling with asar gives 'Cannot find module 0.bundle.worker.js' error

dinataranis opened this issue Β· 33 comments

Hi!

I am building an electron application with electron-webpack and electron-builder.
I am trying to use threadsjs.
When testing in dev mode everything works great, but unfortunately when trying to pack the app (using electron-builder), with the asar option, I am getting the following error:
`Cannot find module 0.bundle.worker.js``

I think it is caused by a mismatch in the worker's path.
Since packing with asar option causes the entire app to be bundled as a single file, it can't access the worker at runtime.

After searching the web for a bit I found the asarUnpack option (which specifies which files to unpack when creating the asar archive) is probably what we are looking for, but I think it requires changes in threads-plugin to allow worker files to be bundle as asarUnpack.
Code example with asarUnpack flag:

"asarUnpack": [
  "dist/main/0.bundle.worker.js",
  "dist/main/0.bundle.worker.js.map"
]

I have reproduced the issue with the basic template, it can be accessed from here.
When bundling with asar: false everything works as expected, but without we get the error above.

Bundling with asar: false is strongly not recommended solution, and it really affects the app performance.

We will be thankful if this can be solved.

P.S. I took the dependencies from my current project, but I get the same error, when I use the latest

Best regards.

Hey @dinataranis, thanks for reporting.

How do you think would the solution look like? The threads-plugin cannot set the asarUnpack option as that latter one is in electron-builder.

I could imagine that setting the asarUnpack option manually (could also set it to something generic like dist/main/*.worker.js*, I think) would make the app work. Have you tried yet?

Hi, @andywer, sorry for delay.
I tried to set asarUnpack manually as you advised (it saves *.worker.js* files in app.asar.unpacked instead of packing them in app.asar), but in the release mode I still get the same error Cannot find module ...resources\app.asar\dist\main\0.bundle.worker.js
If I understand it correctly, threads-plugin changes the paths given anywhere I use new Worker('./relative/path/to/worker)
This causes 'threads' to search for my worker in app.asar file. but files cannot be accessed externally within app.asar, so we have to unpack it as you suggested. nevertheless, we still need to notify threads to search in the new path (a.k.a app.asar.unpacked/dist/main/0.bundle.worker.js)
For that I think we need your help

I am not yet 100% sure how the solution eventually needs to look like, but I think we need some way to optionally pass a custom worker-path-rewrite function to the threads-plugin, so you can use something like this.

Hi @andywer,
I took a look into this problem as well, and I think what you've suggested is the right way to go.
something like an options object passed to

new Worker(path, {
    overrideResolvedPath: (resolvedPath:string)=>string;
})

(That's in typescript to make it more clear.)

Just for checking, we have changed the code under implementation.node.js

function resolveScriptPath(scriptPath) {
    // eval() hack is also webpack-related
    const workerFilePath = typeof __non_webpack_require__ === "function"
        ? __non_webpack_require__.resolve(path.join(eval("__dirname"), scriptPath))
        : require.resolve(rebaseScriptPath(scriptPath, /[\/\\]worker_threads[\/\\]/));
    // ~~~~~~~~hardcoded check~~~~~~~~~~~
    return workerFilePath.replace('app.asar', 'app.asar.unpack');
}

And it does find the module, but unfortunately, we get another error:

Cannot find module 'source-map-support/source-map-support.js'

It makes sense since in 0.bundle.worker.js starts like this:

require("source-map-support/source-map-support.js").install(),function(e){var t={};function r(n){if(t[n])return t[n].exports;var o=t[n]={i:n,l:!1,exports:{}};return e[n].call(o.exports,o,o.exports,r),o.l=!0,o.exports}r.m=e,r.c=t,r.d=function(e,t,n){r.o(e,t)||Object.defineProperty(e,t,{enumerable:!0,get:n})},r.r=function(e){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(e,"__esModule",{value:!0})},r.t=function(e,t){if(1&t&&
.... more code

I'm not sure if it's related to threads or threads-plugin though.

Any ideas?

@raz-sinay No, the option should not go into the Worker instantiation, but be passed to the threads-plugin instantiation in the webpack config, so it can rewrite the path at build time.

Might find some time to look into that tonight.

@raz-sinay @dinataranis Can you try to apply this little patch to your node_modules/threads-plugin?

Don't forget to update the dist/threads-plugin.js and set the rewritePath option on new ThreadsPlugin() in the webpack config πŸ˜‰

diff --git a/src/index.js b/src/index.js
index f944e7c..e5515e8 100644
--- a/src/index.js
+++ b/src/index.js
@@ -31,6 +31,7 @@ export default class WorkerPlugin {
 
   apply (compiler) {
     let workerId = 0;
+    const rewritePath = this.options.rewritePath || (workerPath => workerPath);
 
     compiler.hooks.normalModuleFactory.tap(NAME, factory => {
       for (const type of JS_TYPES) {
@@ -74,7 +75,8 @@ export default class WorkerPlugin {
             }
 
             const loaderOptions = { name: opts.name || workerId + '' };
-            const req = `require(${JSON.stringify(workerLoader + '?' + JSON.stringify(loaderOptions) + '!' + dep.string)})`;
+            const workerPath = rewritePath(dep.string);
+            const req = `require(${JSON.stringify(workerLoader + '?' + JSON.stringify(loaderOptions) + '!' + workerPath)})`;
             const id = `__webpack__worker__${workerId++}`;
             ParserHelpers.toConstantDependency(parser, id)(expr.arguments[0]);

Hi, @andywer
Thanks for your update.
I tried your solution, but I got the same error.
As I see in dist/threads-plugin.js we apply the rewritePath function to the worker's relative path instead of absolute

const workerPath = rewritePath('./worker.js');

@dinataranis @raz-sinay Ohh, sh**, you guys are right, of course. Sorry for the confusion…

@dinataranis Can you try monkey-patching implementation.node.js in threads to replace app.asar with app.asar.unpack as Raz did?

@raz-sinay The source-map-support issue seems to be unrelated to threads/threads-plugin. Maybe an npm ls source-map-support explains where it comes from?

Hi, @andywer
I changed the code in implementation.node.js to replace app.asar with app.asar.unpack and got another error:

Cannot find module 'source-map-support/source-map-support.js.

So I added source-map-support and all it's dependencies to the asarUnpack option in package.json and ... it works!

"asarUnpack": [
  "dist/main/*.worker.js*",
  "node_modules/source-map/**/*",
  "node_modules/source-map-support/**/*",
  "node_modules/buffer-from/**/*",
]

It means, that all node modules which I use in workers files I have to include to asarUnpack option.
It will be great if you can fix the workerFilePath path in theradsjs/thereads-plugin modules.
Thank you for you help!

@dinataranis Can you try npm install threads@1.3.1-asar.unpack? It should work without any extra options besides the asarUnpack in the package.json. The node.js workers will automatically try the *.asar.unpack path if the resolved worker path contains .asar/ and instantiation failed.

Ping πŸ‘‰ @dinataranis.

Hi, @andywer
I have tried the threads@1.3.1-asar.unpack version, and I got the same error Cannot find module '...\resources\app.asar\dist\main\0.bundle.worker.js'
You can see it in my updated project here
I debugged it a little and in the release mode in the file threads\dist-esm\master\implementation.node.js in Worker constructor if I print the final path string throw resolvedScriptPath I still see the ...resources\app.asar\dist\main\0.bundle.worker.js

constructor(scriptPath, options) {
    const resolvedScriptPath = resolveScriptPath(scriptPath);
    if (resolvedScriptPath.match(/\.tsx?$/i) && detectTsNode()) {
        super(createTsNodeModule(resolvedScriptPath), { eval: true });
    }
    else if (resolvedScriptPath.match(/\.asar\//)) {
        try {
            super(resolvedScriptPath, options);
        }
        catch (_a) {
            // See <https://github.com/andywer/threads-plugin/issues/17>
            super(resolvedScriptPath.replace(/\.asar([/\/])/, ".asar.unpack$1"), options);
        }
    }
    else {
        super(resolvedScriptPath, options);
    }

    throw resolvedScriptPath;
    
    this.mappedEventListeners = new WeakMap();
    allWorkers.push(this);
}

The small update.
I think that the problem could be in this condition resolvedScriptPath.match(/\.asar\//) in implementation.node.js file.
I have reproduced it in js console and got null

var path = "resources\app.asar\dist\main\0.bundle.worker.js"
path.match(/\.asar\//) 
// null

It is because of the backslashes in the string. If we try to escape them first and only after what make the search, it should work.

var path = "resources\\app.asar\\dist\\main\\0.bundle.worker.js"
path.match(/\.asar[\\,\/]/)

Yeah, you can monkey-patch it for now using .match(/\.asar[\\\/]/). In the .replace() call the regex is correct (it's wrong in a different way, actually), it's wrong in the condition only (don't ask me why…).

Try this commit πŸ˜‰
andywer/threads.js@ab2f3b9

I published the latest feature branch commit using a testing tag. Let me know if this one works for you.

npm install threads@1.3.1-asar.unpack.2

Hi, @andywer
Just a few remarks about 1.3.1-asar.unpack.2 version.
When I have installed it

"dependencies": {
    "threads": "1.3.1-asar.unpack.2",
    ...
}

I didn't get your fresh changes, but I just added them manually (changed the match condition to /\.asar[\/\\]/, replace condition to (/\.asar([\/\\])/ and instead of .asar.unpack$1 should be .asar.unpacked$1) and it works as expected!!

@dinataranis Ohhh. That was me being stupid while publishing. Can you give 1.3.1-asar.unpack.3 a try? That one the latest feature branch commit for real.

Just a tiny thing, you have forgotten to change .asar.unpack$1 to .asar.unpacked$1

@andywer Was that merged to some version we can use?

Ohh man. I was so busy, I completely forgot. It's merged and published as threads.js v1.4.0 now! πŸ™Œ

I think something is wrong with the regex. because I'm still getting:
Cannot find module 'path\to\resources\app.asar\dist\main\1.bundle.worker.js'

@raz-sinay To be on the safe side, can you list the following information?

  • Your node.js version
  • The version in your node_modules/threads/package.json
  • Is tiny-worker installed?
  • You are using Windows, right?

Sure,

  • NodeJS version is integrated within electron . So, electron version is 7.1.14 which contains NodeJS 12.8.1
  • node_modules/threads/package.json - 1.4.0,
  • tiny-worker isn't installed and wasn't installed before also.
  • I am using windows.

When I test the path against the regex /\.asar[\/\\]/ It seems that it doesn't match because the path is given with a single \. I mean: C:\path\to\worker
when I escape it to C:\\path\\to\\worker the regex above does find a match. I'm not sure that's the issue, but it might help investigating so i'm mentioning it.

Not sure if it's the regex. I just quickly tried it in a REPL and it matched:

> "C:\\path\\to\\resources\\app.asar\\dist\\main\\1.bundle.worker.js".match(/\.asar[\/\\]/)
[".asar\", index: 24, input: "C:\path\to\resources\app.asar\dist\main\1.bundle.worker.js", groups: undefined]

Did you set the asarUnpack property in the package.json?

I did, also made sure that it was unpacked.
Did it work for you even with a single \ ? I mean unescaped path?

@andywer We did some more investigation.
found out that:

// node_modules\threads\dist-esm\master\implementation.node.js
 class Worker extends NativeWorker {
        constructor(scriptPath, options) {
            const resolvedScriptPath = resolveScriptPath(scriptPath, (options || {})._baseURL);
            if (resolvedScriptPath.match(/\.tsx?$/i) && detectTsNode()) {
                super(createTsNodeModule(resolvedScriptPath), Object.assign(Object.assign({}, options), { eval: true }));
            }
            else if (resolvedScriptPath.match(/\.asar[\/\\]/)) {
                try {             
                    **** // If I add here throw new Error(), everything works as expected ****
                    super(resolvedScriptPath, options);  // This line doesn't throw error                    
                }
                catch (_a) {
                    // See <https://github.com/andywer/threads-plugin/issues/17>
                    super(resolvedScriptPath.replace(/\.asar([\/\\])/, ".asar.unpacked$1"), options);
                }
            }
            else {
                super(resolvedScriptPath, options);
            }
            this.mappedEventListeners = new WeakMap();
            allWorkers.push(this);
        }

@andywer any news with this one?

@raz-sinay Ohh, sorry.

Did it work for you even with a single \ ? I mean unescaped path?

Don't have a Windows system to test, besides in CI.

I guess we should not catch and then fix the path, but just assume we need to fix it or maybe add an option to control the ASAR path rewriting, but let it default to rewrite.

I guess we should not catch and then fix the path, but just assume we need to fix it or maybe add an option to control the ASAR path rewriting, but let it default to rewrite.

Sounds good.

I opened a follow-up PR andywer/threads.js#236 and published it under a testing tag as threads@1.4.0-resolve-asar-immediately.

@raz-sinay Give it a try – hope that does the job πŸ˜‰

I opened a follow-up PR andywer/threads.js#236 and published it under a testing tag as threads@1.4.0-resolve-asar-immediately.

@raz-sinay Give it a try – hope that does the job

Works!

This should probably be reverted (or optional) now that electron natively supports asar's electron/electron@06a00b7