search

electron / symbolicate-mac

Symbolicate macOS Electron crash reports
ISC License
31 stars 14 forks source link

feat: support symbolicating windows crashes #49

Closed indutny closed 4 months ago

indutny commented 5 months ago

This is a long shot, but since this script already has all necessary tools to symbolicate text versions and the patch is very minimal, I thought we could add support for Windows crashes as well.

We have been using this patched version of symbolicate-mac internally for awhile, and our tooling can format Windows crashes in the way that is compatible with it. Perhaps, it'll be useful for others too!

indutny commented 4 months ago

That was my thought process as well 😅 I feel like renaming or forking the module for the sake of one line change is too much

On Mon, Apr 22, 2024 at 06:18, Samuel Attard @.***(mailto:On Mon, Apr 22, 2024 at 06:18, Samuel Attard < wrote:

@MarshallOfSound commented on this pull request.

This module does have uh, awkward naming to add this as a feature 😆 But the code is so small we could land it 🙃

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

nornagon commented 4 months ago

When you say "our tooling can format Windows crashes in the way that is compatible with it", what tooling is that? Is this a standard crash dump format on Windows?

If it's a standard format, I think this would be okay to merge (and we should maybe rename the module to symbolicate-crash or something). If it's a custom format specific to your tooling then I'm not sure it belongs here :/

indutny-signal commented 4 months ago

Sorry for a delay!

Our pipeline is:

  1. Use minidump crate to parse local dump
  2. Remove everything except stack trace and thread information from the dump
  3. Output it to debug log in json

Then we have a separate script that converts that json to the format that is compatible with symbolicate-mac. So... is it standard? Yeah, sort of! Do we make it happen so - for sure :)

We currently float a patch-package patch to add this one-line change, so I'd love to get us to a place where it isn't necessary. Thanks!

continuous-auth[bot] commented 4 months ago

:tada: This PR is included in version 2.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

indutny-signal commented 4 months ago

Aw, you are the best! Thank you!