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

Add process.setDefaultErrorMode() #7335

Merged
merged 1 commit into from Oct 3, 2016

Conversation

Projects
None yet
3 participants
@miniak
Contributor

miniak commented Sep 24, 2016

This method has the same effect as https://github.com/electron/electron/blob/master/docs/api/environment-variables.md#electron_default_error_mode-windows

This is useful for apps not using the crashReporter module and/or for debugging.

@paulcbetts

This comment has been minimized.

Show comment
Hide comment
@paulcbetts

paulcbetts Sep 24, 2016

Contributor

Instead of creating this new API, can you just call it by default and disable it if users set up the crash reporter?

Contributor

paulcbetts commented Sep 24, 2016

Instead of creating this new API, can you just call it by default and disable it if users set up the crash reporter?

@miniak

This comment has been minimized.

Show comment
Hide comment
@miniak

miniak Sep 24, 2016

Contributor

If the other guys agree, why not.

Contributor

miniak commented Sep 24, 2016

If the other guys agree, why not.

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Sep 26, 2016

Contributor

Instead of creating this new API, can you just call it by default and disable it if users set up the crash reporter?

👍

Contributor

zcbenz commented Sep 26, 2016

Instead of creating this new API, can you just call it by default and disable it if users set up the crash reporter?

👍

@miniak

This comment has been minimized.

Show comment
Hide comment
@miniak

miniak Sep 29, 2016

Contributor

@zcbenz The crash reporter overrides WER when it's initialized, therefore it looks like we can just set the error mode and that's it.

Contributor

miniak commented Sep 29, 2016

@zcbenz The crash reporter overrides WER when it's initialized, therefore it looks like we can just set the error mode and that's it.

@miniak

This comment has been minimized.

Show comment
Hide comment
@miniak

miniak Sep 29, 2016

Contributor

It would make sense to always call SetErrorMode(GetErrorMode() & ~SEM_NOGPFAULTERRORBOX) in the main process. For the renderer, we get the 'crashed' event. In case we would like to still trigger WER, we can just call process.env.ELECTRON_DEFAULT_ERROR_MODE = 1; in the main script

Contributor

miniak commented Sep 29, 2016

It would make sense to always call SetErrorMode(GetErrorMode() & ~SEM_NOGPFAULTERRORBOX) in the main process. For the renderer, we get the 'crashed' event. In case we would like to still trigger WER, we can just call process.env.ELECTRON_DEFAULT_ERROR_MODE = 1; in the main script

@miniak

This comment has been minimized.

Show comment
Hide comment
@miniak

miniak Sep 29, 2016

Contributor

Or if we want to keep the old behavior by default for backwards compatibility, what about adding an optional entry to package.json to set it before the main script runs?

Contributor

miniak commented Sep 29, 2016

Or if we want to keep the old behavior by default for backwards compatibility, what about adding an optional entry to package.json to set it before the main script runs?

@zcbenz

This comment has been minimized.

Show comment
Hide comment
@zcbenz

zcbenz Oct 3, 2016

Contributor

Current change looks good to me. 👍

Contributor

zcbenz commented Oct 3, 2016

Current change looks good to me. 👍

@zcbenz zcbenz merged commit 772c456 into electron:master Oct 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@miniak miniak deleted the miniak:set-default-error-mode branch Oct 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment