Grpc-node: Electron OSX build: grpc is unable to load Google proto files

Created on 12 Jul 2019  Â·  28Comments  Â·  Source: grpc/grpc-node

I'm working on an electron 5.x app.
The dev build works fine, but the prod build is broken.

When I start the app, a blank screen appears and I get this error:

Error: ENOENT: no such file or directory, open 'google/protobuf/api.proto'

This happens when I'm trying to create an OSX build using electron-builder. The build process is successful and there are no errors. The resulting asar file contains the node_modules dir as it should and grpc seems to be installed properly.

But somehow the paths seem to be messed up in the prod build. Any suggestions on what could be wrong? How should I fix this?

Most helpful comment

I ran into this issue as well. I am using Webpack and merely imported @google-cloud/storage and bam! The app broke in production:

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type number (58)
at validateString (internal/validators.js:121:11)
at Object.dirname (path.js:1128:5)
at Object.<anonymous> (/home/xxx/dist/server/index.js:127216:38)

It took quite a while to figure out what's going on.

The simple fix

Here's my simple fix for those who are facing this issue and Google still hasn't fixed the @grpc/proto-loader library, which is a dependency of the @google-cloud/storage.

Update your webpack.config.js to have a new loader (specify it first):

...
  rules: [
    {
       test: /@grpc\/proto-loader/,
       use: path.resolve('scripts/remove-proto-loader-protos.js'),
    },
...

Then create file scripts/remove-proto-loader-protos.js with the following code:

// https://github.com/grpc/grpc-node/issues/969
module.exports = function (source) {
  if (source.includes(`Load Google's well-known proto files`)) {
    return source.replace(/\/\/ Load Google's well-known proto files.*?\n}/gis, '')
  } else {
    return source
  }
}

That's it! The proto loading code will be automatically _wiped out_. No need for separate forks and keeping them updated manually.

This of course means that if anyone needs those proto files, you have to load them manually (look above at @mrfelton's comment). Also, if Google updates their code, this hack may need to be updated in the future. I know, this is a hack, but what can you do? ¯\_(ツ)_/¯

All 28 comments

Any suggestions?

How can I make this work with electron? Actually, this seems to caused by the proto-loader. That's where the error appears.

Hello? Anybody?

My understanding is that this package is supposed to support electron. I just updated to the latest grpc but no change. I can't create preod builds.

Please, any suggestions on what to try? Shouldn't this just work out of the box? I mean, the proto-loader tries to load those files and that's part of the grpc package.

Are you able to inspect this value? The proto-loader loads a handful of well-known proto files from protobufjs inside the node_modules. Can you verify that protobufjs is in your node_modules directory?

@cjihrig

Thx for the response!

The protobufjs is the dependency of grpc. It's located under:

node_modules/@grpc/proto-loader/node_modules/protobufjs

However, when electron-builder creates the app.asar, it does not copy this directory. There is a node_modules in the asar, but @grpc is not there.

Should I add the protobufjs as a direct production dependency?

I don't know enough about electron-builder to comment on that. Excluding dependencies seems like a bug to me though. You could try adding protobufjs as a production dependency to verify that it fixes your issue, but again, I'm not sure what the expected behavior from electron-builder is.

Well, ok thx. EB should copy all production dependencies.
So protobufjs has to be installed in node_modules in order to make this work properly. Thanks!

I tried to install it manually, but EB does not copy it to the asar. So this seems to be an EB problem. Closing this issue and will open another for EB :).

@cjihrig
I've managed to copy protobufjs to the asar. Now I can see it in asar/node_modules:

/node_modules/protobufjs

This file is also available:

/node_modules/protobufjs/google/protobuf/api.proto

But when I start the app, I get the same error! Apparently, it cannot find the the api.proto for some reason:

Full stack trace:

Uncaught Error: ENOENT: no such file or directory, open 'google/protobuf/api.proto'
    at Object.openSync (fs.js:453)
    at Object.func [as openSync] (electron/js2c/asar.js:155)
    at Object.readFileSync (fs.js:353)
    at Object.fs.readFileSync (electron/js2c/asar.js:597)
    at h (root.js:160)
    at h.t [as load] (root.js:194)
    at h.loadSync (root.js:235)
    at Object.s [as loadSync] (index-light.js:69)
    at Object.f193 (index.js:244)
    at n (bootstrap:83)

What do you suggest? Shouldn't this just work as it is? It works fine in dev build...

It works fine in dev build...

Does the prod build delete unused files? If so, the tool might think those proto files are unused and delete them.

Yes It does, but the prod build puts its output elsewhere. Also the prod build creates an electron asar archive and puts all files into that and the dev build has no access to this file.

Have you tried tried debugging the lines I linked to in https://github.com/grpc/grpc-node/issues/969#issuecomment-514643410? The fact that proto-loader is trying to open 'google/protobuf/api.proto' implies that something is going wrong with the the require.resolve('protobufjs') call.

Yeah thanks for pointing that out. I checked it in debugger and indeed, require.resolve appears to return nothing. But the problem is I have no idea what's causing this.
Could it be webpack? I tried to check the prod and ev configs but node settings are the same.

How is this possible, any ideas :)? I mean, electron is node environment, require.resolve should return something, unless it looks for modules in the wrong directory somehow..

Webpack's require.resolve() is different from Node's. Assuming that's the issue, I'd imagine that others have encountered the problem using electron-builder. Can you open an issue there and see what they have to say about it? If there is no workaround, maybe some changes to the proto-loader can be made.

OK I'll try. You mean your require.resolve() call has been replaced by webpack with its own resolve call?
I mean, what you are calling there is clearly the node version.
And why does the dev build work? That's also performed by webpack.. strange..

You mean your require.resolve() call has been replaced by webpack with its own resolve call?

webpack has to shim some of Node's APIs. Since webpack combines multiple files into a bundle, that impacts the way one file imports another. Again, I'm not sure that's the actual problem, and haven't used electron-builder myself, which is I why I recommended asking on that project's issue tracker. But since the dev build works as expected, it's not a clear cut issue with grpc (IMO).

@cjihrig
Can't find anything in electron builder issue tracker. This must be caused by something else.
How can you tell if the require.resolve is webpack's version or the original node func?

I think it's either a webpack (as you described) or maybe it's looking for node_modules elsewhere, so resolve() cannot find any. But this is less likely.

How can you tell if the require.resolve is webpack's version or the original node func?

webpack's version returns a module ID, while the real require.resolve() returns a file system path.

maybe it's looking for node_modules elsewhere, so resolve() cannot find any

If the real require.resolve() can't find a module, it throws.

@cjihrig

The require.resolve() in the proto-loader code returns stuff like this

require.resolve('protobufjs') returns '"fa1c"'

and

require.resolve('fs') resturn '"9b0f"'

So webpack seems to replace require.resolve() with its own implementation and your code assumes that require.resolve() is the node version. This is the problem.

I'm not sure how to proceed now. Is this a webpack config problem or you have to handle this in your code?

@cjihrig
I've investigated this further.

In proto-loader index.js, at line 240 you call require.resolve():

 var sourceDir = path.join(path.dirname(require.resolve('protobufjs')), 'google', 'protobuf');

If this is an electron prod build, you have to call this function instead:

 var sourceDir = path.join(path.dirname(__non_webpack_require__.resolve('protobufjs')), 'google', 'protobuf');

The __non_webpack_require__ points to the unmodified node function.

With this change and few config tweaks the app works in prod. I've tested this on Mac Mojave so far. Will test on Windows too later.

Will you modify the proto-loader code accordingly?

I see that ncc did something similar here. Does that work for other bundlers though? If it only works for webpack, I don't think it's an appropriate change for a project like proto-loader.

There are less hacky ways to solve the problem I think. For example, protobufjs could load and expose those proto files, but they've already stated that they don't want to do that. Another option would be for proto-loader to include the files directly.

At this point, if you can't fix the issue in your application, I'd suggest waiting for one of the grpc maintainers to weigh in here. It's ultimately up to them if any changes will be made in proto-loader.

OK I see. I agree this could be implemented in several ways, but using the webpack variable is not necessarily wrong. For example, you could use it in a conditional way which would allow other bundlers to work properly etc.

Anyway, since I don't have time to wait, I'm gonna just fork the project for now and use my own version until the maintainers introduce a better solution.

Thanks for all your insight and help!

I wanted to chime in on this as the way the popular protos are included in the proto-loader package is causing problems in other projects that get bundled with webpack.

The error I was seeing was:

The "path" argument must be of type string. Received type number

As far as I can tell, this is because of the way webpack statically analyzes the line mentioned above at compile time (the error I'm seeing is thrown by path.dirname).

I was able to fix the build by using a fork of the proto-loader project made by a team that ran into the same issue building with electron and webpack. Their fix was to remove the commonly loaded protos block (commit here) since they don't need them anyway. It seems like it would make more sense (or at least solve these issues) to have these loaded conditionally or at least not at runtime.

FWIW, my error is coming from using a package that uses @grpc/proto-loader as a downstream dependency in a project that uses webpack to bundle lambda services, deploying with zeit's now service. Zeit happens to use webpack to bundle code in the build stage. So, in this case, there's not even flexibility to make changes to the webpack config directly (further discussion here).

@gtamas checkout @ln-zap/proto-loader and see if that works for you as they already made a fork for the same situation (building in an electron + webpack project)

Is it really necessary to hardcode the loading of the google proto files into the proto-loader codebase?

If someone wants to use those definitions can't the do so by loading themselves?

import * as protoLoader from '@grpc/proto-loader'
import * as protoFiles from 'google-proto-files';

protoLoader.loadSync(`service.proto`, {
  includeDirs: [ protoFiles.getProtoPath('..') ]
});

I ran into this issue as well. I am using Webpack and merely imported @google-cloud/storage and bam! The app broke in production:

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type number (58)
at validateString (internal/validators.js:121:11)
at Object.dirname (path.js:1128:5)
at Object.<anonymous> (/home/xxx/dist/server/index.js:127216:38)

It took quite a while to figure out what's going on.

The simple fix

Here's my simple fix for those who are facing this issue and Google still hasn't fixed the @grpc/proto-loader library, which is a dependency of the @google-cloud/storage.

Update your webpack.config.js to have a new loader (specify it first):

...
  rules: [
    {
       test: /@grpc\/proto-loader/,
       use: path.resolve('scripts/remove-proto-loader-protos.js'),
    },
...

Then create file scripts/remove-proto-loader-protos.js with the following code:

// https://github.com/grpc/grpc-node/issues/969
module.exports = function (source) {
  if (source.includes(`Load Google's well-known proto files`)) {
    return source.replace(/\/\/ Load Google's well-known proto files.*?\n}/gis, '')
  } else {
    return source
  }
}

That's it! The proto loading code will be automatically _wiped out_. No need for separate forks and keeping them updated manually.

This of course means that if anyone needs those proto files, you have to load them manually (look above at @mrfelton's comment). Also, if Google updates their code, this hack may need to be updated in the future. I know, this is a hack, but what can you do? ¯\_(ツ)_/¯

I managed to fix the issue with this Webpack config:

externals: {
  '@grpc/proto-loader': 'commonjs @grpc/proto-loader',
},

Can this please be fixed soon? From @mrfelton and @kaisellgren's comments it seems like it's much more trouble than it's worth.

If I understand correctly, @kaisellgren's last comment has a one-line fix for this issue. I suggest using that.

@murgatroid99

The one liner suggested by @kaisellgren doesn't work for me. I've added it to webpack config, but there is no change.
@ln-zap/proto-loader causes other errors.

With webpack 4.42 even my fork doesn't seem to work anymore. Now both require and __non_webpack_require__ are undefined values in this line:

var sourceDir = path.join(path.dirname(require.resolve('protobufjs')), 'google', 'protobuf');

I guess it would be best to solve this in this library instead of messing with these hacks and forks.

Is there a proven, working solution out there?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Harmonickey picture Harmonickey  Â·  6Comments

tvk-codecraft picture tvk-codecraft  Â·  3Comments

Slapbox picture Slapbox  Â·  5Comments

samuela picture samuela  Â·  4Comments

bkw picture bkw  Â·  5Comments