Skip to content

Write crash report in temp directory if writing to CWD failed.#113316

Merged
auto-submit[bot] merged 1 commit intoflutter:masterfrom
chingjun:crash-error-handling
Oct 12, 2022
Merged

Write crash report in temp directory if writing to CWD failed.#113316
auto-submit[bot] merged 1 commit intoflutter:masterfrom
chingjun:crash-error-handling

Conversation

@chingjun
Copy link
Contributor

Based on the code, this was the original intention but is now broken because ErrorHandlingFileSystem throws a ToolExit, but the crash handler is only catching FileSystemException.

Wrap the filesystem calls with ErrorHandlingFileSystem.noExitOnFailure so that it throws the right exception, and update
ErrorHandlingFileSystem to throw FileSystemException instead of the generic Exception, to match the behavior described in the comments.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Based on the code, this was the original intention but is now broken
because `ErrorHandlingFileSystem` throws a `ToolExit`, but the crash
handler is only catching `FileSystemException`.

Wrap the filesystem calls with `ErrorHandlingFileSystem.noExitOnFailure`
so that it throws the right exception, and update
`ErrorHandlingFileSystem` to throw `FileSystemException` instead of the
generic `Exception`, to match the behavior described in the comments.
@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 12, 2022
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, good catch!

@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 12, 2022
@auto-submit auto-submit bot merged commit 944a4c9 into flutter:master Oct 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants