ably / ably-js

Javascript, Node, Typescript, React, React Native client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
314 stars 56 forks source link

When using ably-js in Electron from a renderer process, the renderer process crashes after ~30s #628

Open ajoslin103 opened 4 years ago

ajoslin103 commented 4 years ago

A successful Ably.Realtime.connect() causes whitescreen after 30 Seconds in my electron-boilerplate project

There is no error that I can see happening anywhere -- the app just whitescreens

I'm just doing this:

   this.ablyInstance = new Ably.Realtime(ablyKey);
   this.ablyInstance.connection
      .on('connected', data => console.log('Connected to Ably, data:', data));

19:28:15.830 › Connected to Ably, data: ConnectionStateChange {previous: "connecting", current: "connected"}

And 30 seconds later it whitescreens -- there is no uncaught exception or anything...

If I give it a bad key app does not whitescreen

This must have something to do with Presence -- which I don't care about...

What am I missing?

┆Issue is synchronized with this Jira Bug by Unito

ajoslin103 commented 4 years ago

I am also using Ably.Realtime in my browser app and my AdonisJS (nodeJS) app -- none of them have this trouble....

ajoslin103 commented 4 years ago

It also whitescreens with this: new Ably.Realtime({ key: ablyKey, connect: false });

so maybe it's not persistence...

SimonWoolf commented 4 years ago

There's no such option as connect: false. You may be thinking of autoConnect.

ably-js is a pretty standalone library -- it doesn't interact with the DOM in any way (with the exception of the jsonp transport, which is only used on extremely old transports). There's not really any mechanism for it to make the screen white, other than if an uncaught exception or unhandled promise crashes the renderer process or something.

We do not currently test in electron, so it's definitely possible that we're calling some method or other that's not present in electron. Hard to tell what it could be without trying though. (For one thing, I believe there's multiple different ways you could use ably from electron -- eg the browser version from the chromium processes, or the node version from the node processes). If you could make a minimal reproducible app that demonstrates the issue, I'd be happy to investigate and debug it. Or if you'd prefer to continue debugging yourself, we can answer any questions that come up.

ajoslin103 commented 4 years ago

Thank you for your quick response! I got up early to create just such a repo.

https://github.com/ajoslin103/electron-boilerplate edit the ./config/env_development.json file to add your ablyKey value npm i && npm start after the 6th console log entry - whitescreen

ajoslin103 commented 4 years ago

I used: "npm i ably" since I was going to connect from within a browser window inside electron

I tried using ably from the backend.js and passing the instance via IPC but it didn't connect

I've used "npm i ably" in the browser (an aurelia cli generated w/webpack) and in the backend (an adonisjs nodejs app) and in the electron app -- I didn't know there was a different library for browser vs nodejs

I don't think the whitescreen is itself being caused by ably, I think ably is taking the electron thread out-to-lunch somewhere and getting lost on the way back to the office -- likely because electron is telling it some story and they both loose track of where they are.

The fact that the connection is stable and things are working for 20-30 seconds makes me think it's a Presence thing.

Thanks again !!

ajoslin103 commented 4 years ago

Hello,

Is there any other way I can assist with the process of figuring out this bug?

QuintinWillison commented 4 years ago

Hi @ajoslin103 - thanks for the updates. Looking at your code I can't see anything obvious - it appears pretty innocuous!

My only immediate reaction, without spending the time to get a development environment up to match yours, is that you've not got any callback handlers to log success of failure of asynchronous calls to Ably. Could you add those and see if that reveals anything? See Async API style.

Thanks!

ajoslin103 commented 4 years ago

Well that's where it gets bizarre and intimidating to me -- I don't have to make any calls after a successful connection in order to see the problem. Those calls are only in there to show that the connection is working.

You can actually use this call: new Ably.Realtime(ablyKey) and 25-30 seconds later it whitescreens with zero interaction while you are doing something else or just idling !!

I'm at my wits end here, Ably is working fantastically for me for every other part of this puzzle and I don't want to backtrack to find another option...

My dashboard dev console shows nothing but a client disconnect...

What options to turn on debug do I have with the Realtime client?

ajoslin103 commented 4 years ago

Also -- any NodeJS development environment can run that code -- there is nothing special about it. If you can already run npm then you can build and run my project. It's just a fork of https://github.com/szwacz/electron-boilerplate with the small addition of ably & the key... As long as you're not trying to package the resultant app for a different platform than yours, that's when things get more complicated.

ajoslin103 commented 4 years ago

this is terrible - I can't show to the client tomorrow...

paddybyers commented 4 years ago

Hey @ajoslin103 I'm looking at this. I have no clue what's going on atm, but I do see the problem.

Do you get the devtools disconnection message as well?

paddybyers commented 4 years ago

Also, I saw https://stackoverflow.com/a/56111557

Any ideas from that?

ajoslin103 commented 4 years ago

yes, I do get that disconnect -- that SO thread ends with the main problem: But Electron does not use Firefox. – Nisanio Jan 17 at 18:21

Do you think it's possible that Ably is going into some tight loop concerning the Presence and taking over the thread? Maybe Electron is whitescreening because there is nobody home to render that page?

It's so bizarre that it happens as the code is just waiting after connection -- my normal node and browser processes do not have that problem.

Since Ably doesn't interact with the browser...

Are the presence communications on a different port ? What happens when they are blocked ?

I am only looking to the Presence based on the timing coincidence...

paddybyers commented 4 years ago

I really don't think it can be presence: you're not doing anything with presence. You connect, publish. Presence uses the same connection, and there's nothing that would be triggered after a period of connection. (There's a timer that runs if you are present and become disconnected, but I really don't see how that can be relevant to this issue.)

ajoslin103 commented 4 years ago

There must be some timer somewhere that is causing a bit of code to run -- are all timers bottlenecked in the code somewhere?

Is there any chance for a debug version of the code ? Are there any debug flags I can turn on ?

paddybyers commented 4 years ago

You can add log:{level:4} to the client options. I'm doing that, but I still can't see anything happening at the moment that it crashes.

It happens if there is an idle websocket connection. Without a ws connection it doesn't crash.

It's very puzzling, but from googling around this does seem to be a common issue, without any clear explanation or resolution :( I'll keep looking.

ajoslin103 commented 4 years ago

But what does "idle" mean? I can shorten the timeout to 1 second and it pub/subs for 25-30 seconds and dies -- that can't be considered the same 'idle' as doing nothing for 25-30 seconds

It feels like something is outside the normal code path, supervising the connection -- and it's taking offense at something that doesn't happen

Maybe the log level will give me an idea - I'll try that

SimonWoolf commented 4 years ago

Hello @ajoslin103. Thanks for reporting an issue with ably-js, and providing a reproducible test case.

The cause of the issue appears to be a bug in chromium's setTimeout function when called in an electron renderer (~browser) process from a file evaluated from a custom vm context, which ably-js uses for greater isolation.

For your demo tomorrow, I have prepared a version of ably-js with a workaround for this, which no longer evaluates files from a custom VM context. You can use it by replacing the "ably" line in your package.json with "ably": "^1.1.25-electron-workaround",, then running npm install.

Be warned that this workaround version, unlike the normal version, will litter the global object in that renderer process with some top-level properties (since the code was written under the assumption it would be evaluated in a custom context, it does not bother to avoid that).

This is intended to be a temporary workaround; we will consider how we can best support electron in a more permanent way moving forward.

ajoslin103 commented 4 years ago

Thank you so much for the workaround !! That is Awesome !!

In googling what you found I see:

For anyone still having this issue, I realized that VSCode was adding in import { setTimeout } from 'timers'; automatically and causing this issue. I removed that and problem solved. (from the end of this issue: https://github.com/electron/electron/issues/7079)

I have certainly been slammed by VSCode auto-imports in the past !!

And this is interesting as well

In a renderer process, setTimeout() or window.setTimeout() is a Web API function which returns an integer: Whereas in the main process, setTimeout() is a Node.js global object method (described in Timers) which returns a Timeout object: In order to call the Node.js method from the renderer process, you would have to use Electron's remote getGlobal method; for instance: (from this SO thread: https://stackoverflow.com/questions/54783945/electron-settimeout-returns-number-in-renderer-process-instead-of-timeout-object

Anyway - Thanks A Whole Lot !!

SimonWoolf commented 4 years ago

Yup, using the node setTimeout function instead of using chromium's was the other workaround we were considering; we decided to go with the runInThisContext workaround for now.

Another solution would be to use ably-js from the main process, not the renderer process. The main process is a real node.js process, so would not be have the same issue.

I've created a support article for this: https://support.ably.io/en/support/solutions/articles/3000096728-when-using-ably-js-in-electron-from-a-renderer-process-the-renderer-process-crashes-after-30s . I am leaving this issue open until we have a permanent solution for running ably-js in a renderer process.

ajoslin103 commented 4 years ago

I tried using ably in the main process and using Electrons IPC for communication - it did not fare well and the debugger support out there was not good

There is an article on Electron that says do not use that thread for anything but window management, but I can’t find it now

The article said that the main process is too easily blocked and does not have the access to NodeJS modules that the renderer processes do

On Feb 6, 2020, at 7:22 AM, Simon Woolf notifications@github.com wrote:

Yup, using the node setTimeout function instead of using chromium's was the other workaround we were considering; we decided to go with the runInThisContext workaround for now.

Another solution would be to use ably-js from the main process, not the renderer process. The main process is a real node.js process, so would not be have the same issue.

I've created a support article for this: https://support.ably.io/en/support/solutions/articles/3000096728-when-using-ably-js-in-electron-from-a-renderer-process-the-renderer-process-crashes-after-30s https://support.ably.io/en/support/solutions/articles/3000096728-when-using-ably-js-in-electron-from-a-renderer-process-the-renderer-process-crashes-after-30s . I am leaving this issue open until we have a permanent solution for running ably-js in a renderer process.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ably/ably-js/issues/628?email_source=notifications&email_token=AADML5PWOYXTJZMZDNPWJVTRBP6IZA5CNFSM4KO5GQMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7BBSQ#issuecomment-582881482, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADML5IWSKIPTVE5PETAAITRBP6IZANCNFSM4KO5GQMA.

ajoslin103 commented 4 years ago

FWIW: I created a dirt-simple developer utility for sending messages

https://github.com/ajoslin103/ably-node-util

SimonWoolf commented 4 years ago

@ajoslin103 Cool! One suggestion: for one-off publishes I would strongly suggest using Ably.Rest rather than Ably.Realtime. Setting up a websocket connection only to do one publish and exit immediately is not very efficient; it'd be quicker and easier just to fire off a REST publish. (And no need to do your own abortTimer, the library has its own per-request timeout for rest requests)

ajoslin103 commented 4 years ago

I agree the Rest would be better for publishes

I expect the next update to it will support subscribes

I don’t think they can be done w/Rest, am I right ?

On Feb 7, 2020, at 12:48 PM, Simon Woolf notifications@github.com wrote:

@ajoslin103 https://github.com/ajoslin103 Cool! One suggestion: for one-off publishes I would strongly suggest using Ably.Rest rather than Ably.Realtime. Setting up a websocket connection only to do one publish and exit immediately is not very efficient; it'd be quicker and easier just to fire off a REST publish. (And no need to do your own abortTimer, the library has its own per-request timeout for rest requests)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ably/ably-js/issues/628?email_source=notifications&email_token=AADML5M7RE7OIDBEXRHXLN3RBWNFPA5CNFSM4KO5GQMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELD6BDA#issuecomment-583524492, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADML5NXLMUKB7W2TFAKERLRBWNFPANCNFSM4KO5GQMA.

SimonWoolf commented 4 years ago

correct.

(Happy to discuss this further but if you wouldn't mind doing it through other channels, cf ably.io/contact, so we can leave this github issue for discussion of the electron renderer process bug specifically)

QuintinWillison commented 4 years ago

This issue remains open but I wanted to add a quick comment by way of breadcrumbs for next time we take a look at it, in light of the conversation @SimonWoolf and I have just had.

The work that Simon did to create the 1.1.25-electron-workaround cannot be merged to master because it pollutes the global namespace with, in Simon's words:

all the 'internal' globals ably-js uses

The proper solution is to make #477 happen.