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
electron - limit crash reporter to few insiders users #132961
Conversation
product.quality === 'insider' && // only for insiders | ||
typeof crashReporterId === 'string' && // only if we have a crash reporter ID | ||
uuidPattern.test(crashReporterId) && // only if the crash reporter ID is valid | ||
crashReporterId.endsWith('0') // only if the crash reporter ID ends with `0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a heuristic I picked, but maybe you have a better idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepak1556 I ran some experiments generating 30000
UUIDs to roughly simulate our number of present insider users and typically end up with a number around ~1900
that match. Would you think that number if sufficient to find crashes?
crashReporterStartOptions.uploadToServer = true; | ||
const crashReporterDirectory = this._environmentService.crashReporterDirectory; | ||
const crashReporterId = this._environmentService.crashReporterId; | ||
if (crashReporterDirectory || crashReporterId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now using these properties as a hint whether crash reporting is enabled at all.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussions pending offline related to some logistic concerns with disabling the reporter in stable.
This PR fixes #120902 by limiting the crash reporter to:
0