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

After enabling ANR detection for the main process, all Electron processes are spawned twice #784

Closed
3 tasks done
beyondkmp opened this issue Nov 6, 2023 · 8 comments
Closed
3 tasks done
Assignees

Comments

@beyondkmp
Copy link

beyondkmp commented Nov 6, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Electron SDK Version

4.14.0

Electron Version

27.0.3

What platform are you using?

Windows

Link to Sentry event

No response

Steps to Reproduce

After enabling ANR detection for the main process, all Electron processes are being repeatedly created.

image

This ANR detection relies on the sentry/node ANR module, which checks and spawns a child process during application startup. However, this approach is causing other child processes to be duplicated in Electron.

Below is my code. Could you please help me check whether the issue is with my code or if this feature itself has a problem?
https://github.com/beyondkmp/Sentry-electron-anr

Expected Result

Only one additional ANR detection process is spawned

Actual Result

All processes are spawned twice

@beyondkmp beyondkmp changed the title There After enabling ANR detection for the main process, all Electron processes are being repeatedly created Nov 6, 2023
@beyondkmp beyondkmp changed the title After enabling ANR detection for the main process, all Electron processes are being repeatedly created After enabling ANR detection for the main process, all Electron processes are spawn twice Nov 6, 2023
@beyondkmp beyondkmp changed the title After enabling ANR detection for the main process, all Electron processes are spawn twice After enabling ANR detection for the main process, all Electron processes are spawned twice Nov 6, 2023
@fredyangRC
Copy link

The ANR monitoring was a feature we eagerly want to have.
Big thanks to the team effort that made that happen! However we also got a blocking issue.
I hope that you can help facilitate the priority for this fix.

Thanks,
Fred

@lforst
Copy link
Member

lforst commented Nov 8, 2023

Thanks for reporting this. We will take a look!

@timfish
Copy link
Collaborator

timfish commented Nov 15, 2023

this approach is causing other child processes to be duplicated in Electron

How are you starting other child processes? I looked at the demo repo and couldn't find anything obvious.

Ah ok, looking at the screenshot, there are multiple Electron processes?

@beyondkmp
Copy link
Author

Yes, there are two electron instance. Then there are duplicate network/gpu/crashpad process.

@beyondkmp
Copy link
Author

beyondkmp commented Nov 22, 2023

@timfish
We need to add ELECTRON_RUN_AS_NODE =1 env option in this line(https://github.com/getsentry/sentry-javascript/blob/develop/packages/node/src/anr/index.ts#L106), then this anr process is just a node process instead of multiple electron process.

After adding this environment variable, the icon on the macOS dock no longer flashes. This code(#777) can be removed.

@timfish
Copy link
Collaborator

timfish commented Nov 24, 2023

There are issues with using a node process for ANR, rather than another electron main process:

  • We currently run the existing app main entry point again with a flag. This code (including our SDK) imports electron which will fail in node mode.
  • Without access to Electron APIs we can't resolve much of the app context including the default appname@version release name.

I'll be back from vacation shortly and I'll be able to look into this further.

@timfish
Copy link
Collaborator

timfish commented Dec 24, 2023

@sentry/node now uses a worker thread rather than a child process. Once we get the Electron SDK using the latest JavaScript SDKs this upgrade should be supported in Electron too.

@timfish
Copy link
Collaborator

timfish commented Jan 5, 2024

This was closed by #810 and will be in the next release.

@timfish timfish closed this as completed Jan 5, 2024
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

4 participants