Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client not receiving response when run inside a timeout function or event handler in electron renderer process #26838

Closed
3 tasks done
nateshmbhat opened this issue Dec 5, 2020 · 11 comments

Comments

@nateshmbhat
Copy link

nateshmbhat commented Dec 5, 2020

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

When i run a grpc server in electron , if i send a response after a "timeout" , then the response is not going to the client at all. It's not closing the http2 stream properly. The same server works fine when run in node directly but giving this issue when run through electron.

  • Electron Version:
    11.0.3
  • Operating System:
    Linux pop-os 5.8.0-7625-generic #26~1603389471~20.04~f6b125f-Ubuntu SMP Thu Oct 22 20:35:49 UTC x86_64 x86_64 x86_64 GNU/Linux

Expected Behavior

when the grpc server sends the response with or without the timeout , the response should reach the client and the associated streams should close properly.

Actual Behavior

Though the server sends the response, the response is not reaching the client. So, the connection and stream don't close.

To Reproduce

Sample minimal electron project which demonstrates this issue : https://github.com/nateshmbhat/grpc-with-electron-sample-project

setup of my project
If you provide a URL, please list the commands required to clone/setup/run your repo e.g.

$ git clone https://github.com/nateshmbhat/grpc-with-electron-sample-project -b main
$ npm install
$ npm run dev

Use a grpc client like "bloomrpc" to make grpc call to the server by calling the "SayHello" rpc.

You can see that the response don't come back to the client and the client keeps loading though the server has already sent the response. This issue happens only when we send the response after a timeout (no matter how small that time is) , but doesn't happen when i send the response directly without any timeout.

I have also filed an issue in grpc node repository about this bug here :

grpc/grpc-node#1645

Apparently , i am not facing any issue when using just node (works fine in both node 10 and node 14 on which i tested). But this issue happens only from within electron.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Dec 6, 2020

Your minimal Electron project is far from minimal (TypeScript, Webpack, nested npm scripts etc.). This makes it very unlikely that someone from the Electron team will take a deep look into this (it'll take more time understanding your setup than understanding the actual issue). I'm using @grpc/grpc-js as well, that's why I'm answering. From a brief look at your example is it correct that you are using grpc inside the renderer and not inside the main process? This is important information. Does the issue happen in the main process as well? Maybe you can create a true minimal example using Fiddle or https://github.com/electron/electron-quick-start/

Edit: Also your steps to reproduce are quite involved. E.g.

Use a grpc client like "bloomrpc" to make grpc call to the server by calling the "SayHello" rpc.

Maybe your minimal example could ship with a grpc client (e.g. in the same process) so it is self contained and no external setups are required?

@nateshmbhat
Copy link
Author

nateshmbhat commented Dec 6, 2020

@Prinzhorn thanks for the reply.

I have updated the sample project and removed all the unnecessary nesting and other files from there. I have also added sample client code. You can run the client by running npm run run-client ( have included the instruction in the readme of repo) too.

waiting eagerly for your reply on this : )

Notice the logs in the console. When i run the callback function , though the response get sent ( after the timeout of 10 ms ) , the client takes a very long time to get the response ( client never receives response in bloomrpc actually).

If you run the response callback function without a timeout , it works as expected and the client gets the response immediately.
Let me know if you need anything else from me.

PS : I tried to create a project from electron quick start as you suggested but it gave me errors when loading the proto with errors in resolving the fs module (though i turned on nodeIntegration) . So i decided to fully strip down the above project that i shared to you to make it minimal.

@Prinzhorn
Copy link
Contributor

Prinzhorn commented Dec 6, 2020

I can at least reproduce it and confirm that it works when running node index.js directly (had to update the import/export to make it run).

Also setImmediate instead of setTimeout works as expected.

Also making more than one call and mixing sync and async works as well, e.g. this will successfully log two responses:

/**
 * Implements the SayHello RPC method.
 */
let counter = 0;

function sayHello(call, callback) {
  counter++;
  console.log('Request : ', counter, call);

  if (counter === 1) {
    setTimeout(() => {
      callback(null, { message: 'Hello ' + call.request.name });
    }, 10);
  } else if (counter === 2) {
    callback(null, { message: 'Hello ' + call.request.name });
  }
}
function main() {
    var client = new hello_proto['Greeter']('localhost:50053', grpc.credentials.createInsecure());

    client.sayHello(
        {
            name: 'request',
        },
        function (err, response) {
            console.log('Greeting:', response.message);
        }
    );

    client.sayHello(
        {
            name: 'request',
        },
        function (err, response) {
            console.log('Greeting:', response.message);
        }
    );
}
$ node grpc-client.js

Greeting: Hello request
Greeting: Hello request

Edit: It also only affects the renderer process and not the main process. It works when doing electron index.js directly.

@Prinzhorn
Copy link
Contributor

Running GRPC_TRACE=all GRPC_VERBOSITY=DEBUG npm run run-client reveals that the client actually doesn't receive anything (left is the broken one)

Selection_735

@nateshmbhat
Copy link
Author

any idea as to why this might be happening and what i can do to overcome this issue ? @Prinzhorn

@nateshmbhat
Copy link
Author

any tip would be really helpful ... our team is fully blocked because of this issue.

@zcbenz zcbenz added the bug 🪲 label Dec 9, 2020
@zcbenz
Copy link
Member

zcbenz commented Dec 9, 2020

We don't have a deep understanding of grpc, so we are really not able to look into this bug unless you can find out the root reason yourself.

For your issue, my recommendation is to use child_process.fork to run the script in a separate node process, which should behave the same to the upstream Node.js.

@nateshmbhat nateshmbhat changed the title http2 streams in grpc are not closing properly in electron client not receiving response when run inside a timeout function or event handler in electron renderer process Dec 10, 2020
@nateshmbhat
Copy link
Author

To check if it has anything to do with Timeout , i ran the callback function inside the "event handler" callback of an event emitter. This also produces the same issue.

It looks like the issue is present in the way network calls are getting scheduled in renderer process (just a rough guess)

@Prinzhorn
Copy link
Contributor

@nateshmbhat I'd probably have a fun time tracking this down, but it doesn't affect me. I'm using grpc in a worker thread spawned from the main process. I treat Electron like every other client/server architecture and don't use grpc or anything inside the renderer. In the future I recommend never ever using nodeIntegration and basically run close to no code at all inside the renderer or main process.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Oct 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2022

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants