Crash handling#95
Open
graeme wants to merge 11 commits into
Open
Conversation
Surface the GitHub issue tracker directly from Help so users can file a bug or feature request without hunting for the repository URL. Points at the new-issue page for Homebrew/BrewUI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce a self-contained package that captures and surfaces crashes: - CrashReportInstaller installs an uncaught-exception handler plus POSIX signal handlers for the common fatal signals. The signal path is kept as async-signal-safe as is practical from Swift — the file path and header are materialised at install time, and at crash time it only open/write/backtrace_symbols_fd/close before re-raising the default handler so the OS still records its own report. - CrashReportStore persists reports as plain crash-<epochMillis>.log text files under Application Support/Brew/CrashReports and reads them back (deliberately plain text, not a Codable model, since the artifact is pasted verbatim into an issue). - CrashReportFormatter builds the report text; CrashReportIssue builds a pre-filled GitHub new-issue URL with the log embedded and truncated to stay under GitHub's URL length ceiling. - CrashReportController (@mainactor @observable) loads pending reports on launch and drives the "report it" / "discard" UI. Full unit coverage for the store, formatter, issue-URL builder, report summary, and controller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Install crash capture at the very start of app launch and present any report left by a previous launch: - BrewApp installs CrashReportInstaller before other startup work and owns a CrashReportController seeded from the same store. - CrashReportDialog shows the captured report and offers "Report on GitHub…" (opens the pre-filled issue) or "Discard"; nothing is sent unless the user opts in. - The crashReportSheet view modifier loads pending reports on launch and presents them one at a time, advancing through the queue as each is handled. - Link the BrewCrashReporting product into the Homebrew app target. The app target's build-tool lint plugin can't be exercised via xcodebuild in this environment, so app sources were validated with swiftc -typecheck against the built package modules (0 errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DEBUG-only submenu that deliberately crashes the app so the crash reporting flow can be exercised end to end — the report should surface on the next launch. Covers both capture paths: fatalError (signal handler) and a raised NSException (uncaught-exception handler). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The archive already produces Homebrew.app.dSYM (Release is dwarf-with-dsym) but the build discarded it, leaving shipped crashes unsymbolicatable. Zip the archive's dSYMs, upload them as a build artifact, and attach them to the GitHub release alongside the pkg/app (and attest their provenance). One set covers both delivery vectors — the .pkg and .zip ship the same Homebrew.app binary from a single archive, so the dSYM UUID matches either. dSYMs download into their own dir in the release job so they don't get picked up by the dist/Homebrew-*.zip app-zip glob. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keep short doc comments on public types, the async-signal-safety notes, and the write-once justification for the global handler state. Drop comments that merely restate a name or narrate history, especially on private declarations where the implementation is self-explanatory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new crash-reporting module (BrewCrashReporting) that captures fatal signals/uncaught exceptions to disk and, on the next launch, presents a dialog that helps the user file a pre-filled GitHub issue. It also updates the release pipeline to ship dSYMs so collected crash logs can be symbolicated.
Changes:
- Introduces
BrewCrashReporting(store/formatter/installer/issue-url builder/controller) plus a new test suite covering core behavior. - Wires crash capture into app startup and adds SwiftUI sheet + dialog UX, plus DEBUG-only “Force Crash” menu actions.
- Extends CI/release workflows to zip/upload dSYMs and attach them to GitHub releases.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/BrewCrashReportingTests/CrashReportTests.swift | Adds unit tests for crash report summary extraction. |
| Tests/BrewCrashReportingTests/CrashReportStoreTests.swift | Adds unit tests for on-disk persistence, ordering, and deletion behavior. |
| Tests/BrewCrashReportingTests/CrashReportIssueTests.swift | Adds unit tests for “new issue” URL construction and truncation behavior. |
| Tests/BrewCrashReportingTests/CrashReportFormatterTests.swift | Adds unit tests for report header/body formatting. |
| Tests/BrewCrashReportingTests/CrashReportControllerTests.swift | Adds unit tests for controller queue behavior and disk deletion. |
| Sources/BrewCrashReporting/CrashReportStore.swift | Implements persistence and enumeration of crash report files. |
| Sources/BrewCrashReporting/CrashReportIssue.swift | Builds pre-filled GitHub issue URLs from a crash report. |
| Sources/BrewCrashReporting/CrashReportInstaller.swift | Installs uncaught-exception and fatal-signal handlers and writes reports. |
| Sources/BrewCrashReporting/CrashReportFormatter.swift | Formats crash report text for both signal and exception paths. |
| Sources/BrewCrashReporting/CrashReportEnvironment.swift | Captures app + OS environment metadata for inclusion in reports. |
| Sources/BrewCrashReporting/CrashReportController.swift | Manages pending report queue for UI presentation. |
| Sources/BrewCrashReporting/CrashReport.swift | Defines the crash report model and title/summary derivation. |
| Package.swift | Registers the new SwiftPM target/product and its test target. |
| Homebrew/Features/CrashReporting/CrashReportSheet.swift | Adds SwiftUI wiring to present crash reports as a sheet. |
| Homebrew/Features/CrashReporting/CrashReportDialog.swift | Implements the “report/discard” crash dialog UI. |
| Homebrew/Debug/DebugMenuCommands.swift | Adds DEBUG-only actions to intentionally crash for manual verification. |
| Homebrew/BrewApp.swift | Installs crash capture at app init and adds Help menu “Report an Issue…”. |
| Homebrew.xcodeproj/project.pbxproj | Links the new BrewCrashReporting product into the app target. |
| .github/workflows/build.yml | Zips dSYMs and uploads them as a build artifact during notarized builds. |
| .github/workflows/release.yml | Downloads dSYMs and attaches them to the created GitHub release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Address four Copilot review comments: - Make the crash-sheet's `sheet(item:)` binding setter a no-op. Both it and the dialog's onDismiss discarded the current report, so a SwiftUI or system-initiated nil-set could delete the head of the queue with no user choice. Presentation is already driven by the getter going nil after an explicit discard. - Precompute the report header's byte length at install time instead of calling strlen (not async-signal-safe) inside the signal handler. - Pre-allocate the backtrace frame buffer at install time and drop withUnsafeTemporaryAllocation from the handler, which could otherwise fall back to malloc in the signal context. - Install signal handlers via sigaction with SA_RESETHAND | SA_NODEFER and re-raise, rather than signal()/SIG_DFL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The release dry run (push trigger) promotes the latest main build, which predates the dSYMs artifact until this change ships. Downloading it under set -e aborted the run, so editing release.yml turned the PR red. Make the dSYMs download best-effort and require the artifact only on a real release from main; a dry run now warns and proceeds without it. Once this merges, every main build produces dSYMs, so real releases keep them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Basic crash handling with a next-launch report dialog
Summary
Adds lightweight crash capture to the Homebrew app. When the app crashes, a
report is written to disk; on the next launch the user is offered a pre-filled
GitHub issue so the crash can be reported in one click. Nothing is transmitted
automatically.
Changes
Core (
BrewCrashReporting, new SwiftPM target):CrashReportInstallerinstalls an uncaught-exception handler plus POSIXsignal handlers (SIGABRT/BUS/FPE/ILL/SEGV/TRAP). The signal path is
async-signal-safe: the file path and header are materialised at install time,
and the handler only does
open/write/backtrace_symbols_fd/closebeforere-raising so the OS still records its own report.
CrashReportStorepersists reports ascrash-<epochMillis>.logtext files;CrashReportFormatter/CrashReportEnvironmentbuild the report body withapp version and OS;
CrashReportIssuebuilds the pre-filled new-issue URL;CrashReportControllerdrives the UI state.App wiring:
BrewApp.init();CrashReportSheetpresents
CrashReportDialogfor any pending reports one at a time.Release:
Homebrew.app.dSYMis now zipped, uploaded, and attached to theGitHub release so shipped crashes are symbolicatable (one set covers both the
.pkg and .zip, which share a single archive).
Why this split
Stacked on the release-pipeline branch; the dSYM change belongs here because it
makes captured crashes actionable.
Testing
scripts/test— 19 newBrewCrashReportingunit tests pass.swiftformat --lintandswiftlint --strictclean on changed scope.PR checklist
performed: Claude Code implemented the feature, tests, and release changes;
the author reviewed the diff, ran the local checks above, and manually
verified the crash-to-report flow in a DEBUG build.