Skip to content

[flutter_tools] Handle errors on the std{out,err}.done future#51660

Merged
fluttergithubbot merged 1 commit intoflutter:masterfrom
zanderso:stdio-zone
Feb 29, 2020
Merged

[flutter_tools] Handle errors on the std{out,err}.done future#51660
fluttergithubbot merged 1 commit intoflutter:masterfrom
zanderso:stdio-zone

Conversation

@zanderso
Copy link
Copy Markdown
Member

@zanderso zanderso commented Feb 28, 2020

Description

stdout and stderr can put errors on their respective done futures. This PR catches those errors to avoid the tool crashing on a write to stdout or stderr.

Related Issues

Crash seen in crash logging.

Tests

I added the following tests:

Added a test logger_test.dart

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

  • No, no existing tests failed, so this is not a breaking change.

@zanderso zanderso added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Feb 28, 2020
Copy link
Copy Markdown
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM with nits

For my own understanding: something goes wrong (like a broken pipe?) and stdout and/or stderr signal this by completing done with an exception. If we write to them after this point, we get a different exception?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this pattern

Comment thread packages/flutter_tools/lib/src/base/io.dart Outdated
Comment thread packages/flutter_tools/test/general.shard/base/logger_test.dart Outdated
Comment thread packages/flutter_tools/lib/src/base/io.dart Outdated
@zanderso
Copy link
Copy Markdown
Member Author

For my own understanding: something goes wrong (like a broken pipe?) and stdout and/or stderr signal this by completing done with an exception. If we write to them after this point, we get a different exception?

Right, we'd get a StateError. I added some comments to explain.

@fluttergithubbot fluttergithubbot merged commit 914bd76 into flutter:master Feb 29, 2020
@zanderso zanderso deleted the stdio-zone branch March 12, 2020 19:44
@lock
Copy link
Copy Markdown

lock Bot commented Apr 2, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@lock lock Bot locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants