VSCodium / vscodium

binary releases of VS Code without MS branding/telemetry/licensing
https://vscodium.com
MIT License
25.05k stars 1.07k forks source link

Crashpad disabled regardless of `enable-crash-reporter` argv #1832

Open AgFlore opened 6 months ago

AgFlore commented 6 months ago

Describe the bug Due to the patch introduced in #787 , regardless of the enable-crash-reporter argv, the crash-reporter is always disabled.

Expected behavior

  1. The enable-crash-reporter argv is supposed to be the rightful controller over whether configureCrashReporter should be run. (check https://github.com/microsoft/vscode/blob/8a19756adf1b689e4f490a871d5067e6c54e3d01/src/main.js#L83C54-L83C75 )
  2. If user enable-crash-reporter, the directory set for crashDumps (%appdata/VSCodium/Crashpad by default) should be initilized, containing an empty settings.dat and two directories attachments and reports. Minidumps are to be placed in the reports dir lest a crash happens someday.

My Proposals

  1. The original problem that lead to #787 patch had been resolved by https://github.com/microsoft/vscode/commit/65475dc56e04da109cf3744987f2e00fafca914f . Should submitURL be empty or undefined, uploadToServer would be false, making it acceptable to crashReporter.start(). So we might as well remove the crash-reporter.patch.
  2. Document the --crash-reporter-directory <absolute-path> cli option somewhere in the codium repository (based on infomation from vscode), so that users can have a better understanding of the crash telemetry, and quickly get to how to get dumps when electron crashes.
    • From my opinion, this single cli option enables crash dump and disables crash telemetry at the same time, and is very worth recommending to our users.
    • This infomation will also be especially helpful for users who have to stick on an earlier version of codium (permanently affected by the 787-patch!) due to compatibility reasons but also looking for minidumps.
  3. For the current documents, the mentioning of telemetry.enableCrashReporter has been obselete (and potentially misleading!) since the one-time transition in v1.49, and should be updated.

Please confirm that this problem is VSCodium-specific

Please confirm that the issue/resolution isn't already documented

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment, and we'll keep it open. If you have any new additional information, please include it with your comment!

AgFlore commented 2 weeks ago

i believe the issue and the solution still validates.