opened
ramya-rao-a
username_0
Start crash reporter inside child processes
issue
created
bpasero
username_1
@username_0 I suggest to just introduce `process.env` variables for the crash reporter metadata and read them in the forked process. Using the local storage as a way to communicate data is a bad hack. At the very minimum it should be a ICrashReporterService that we can require in places where we need it (except for forked processes where we do not have services). Anyway, using process environment would save you that issue as well as you would not have to pass arguments around.

We still only have one crash reporter process even when we start it from other processes, right?
comment
created
ramya-rao-a
username_0
In Mac which uses crashpad yes. There is a single handler for monitoring crashes for all processes (main, renderer, child)

In Windows, we need start another process that does the monitoring. This PR doesn't include that. See https://github.com/electron/electron/blob/master/docs/api/crash-reporter.md#crashreporterstartoptions

I'll have to dig a little more to get the more details
comment
created
bpasero
username_1
@username_0 what we could also do as a starter is to only enable this on macOS for e.g. the extension host.
comment
created
ramya-rao-a
username_0
@username_1 PR updated as we discussed
comment
created
bpasero
username_1
@username_0 I pushed some refactorings and a bugfixes on top of your commit to this PR, please have a look. When trying this out locally I was getting a crash right on startup on the extension host. 

One thing to look into is your use of `crashesDirectory`. Shouldn't this property be set on the crash reporter options directly and not the extra bag? And then, we should probably not set this option all the time but only from the object returned from `getChildProcessStartOptions`
comment
created
ramya-rao-a
username_0
Ideally yes. This option is not used by the main/renderer process, so I didn't see the harm in sending it all the time rather than just for the child process. Fixed with 95c851784cbc61ad86cdd3abc7393f708477a330

btw I spent a lot of time getting the service plumbing to work, but kept getting unhelpful errors while instantiating the  ExtensionHostProcessWorker which I then narrowed down to my service not defining "_serviceBrand". Do we have any docs around do's don'ts for such cases?
comment