Repository navigation
[flutter_tools] Safely handle non-JSON messages in test stream parsers - #192089
Merged
auto-submit[bot] merged 2 commits intoSep 4, 2026
Merged
auto-submit[bot] merged 2 commits into
auto-submit[bot] merged 2 commits into
Conversation
When communicating with remote test runners or browsers over test streams or WebSockets, unhandled non-JSON strings (such as dynamic library loading errors or missing snapshot warnings) caused _ChunkedJsonParser to throw an unhandled FormatException at character 1, crashing the tool. This changes pipeHarnessToRemote in flutter_platform.dart and BrowserManager in flutter_web_platform.dart to intercept non-JSON payloads, log warnings and traces, and ignore invalid frames rather than crashing the tool process. Fixes flutter#191898
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces safe JSON decoding for test runner and browser WebSocket communication in flutter_platform.dart and flutter_web_platform.dart by catching FormatException and logging warnings instead of failing, and adds a corresponding unit test. The review feedback suggests simplifying these stream transformations using Stream.map and Stream.handleError to improve readability and maintainability.
Comment on lines
796
to
826
| final safeJsonTransformer = StreamChannelTransformer<Object?, Object?>( | ||
| StreamTransformer<Object?, Object?>.fromHandlers( | ||
| handleData: (Object? data, EventSink<Object?> sink) { | ||
| if (data is String) { | ||
| try { | ||
| sink.add(json.decode(data)); | ||
| } on FormatException catch (err) { | ||
| _logger.printWarning( | ||
| 'Received unexpected non-JSON message from browser WebSocket: $data', | ||
| ); | ||
| _logger.printTrace('JSON decode error: $err'); | ||
| } | ||
| } else { | ||
| sink.add(data); | ||
| } | ||
| }, | ||
| ), | ||
| StreamSinkTransformer<Object?, Object?>.fromHandlers( | ||
| handleData: (Object? data, EventSink<Object?> sink) { | ||
| sink.add(json.encode(data)); | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| // Whenever we get a message, no matter which child channel it's for, we know | ||
| // the browser is still running code which means the user isn't debugging. | ||
| _channel = MultiChannel<dynamic>( | ||
| webSocket.cast<String>().transform(jsonDocument).changeStream((Stream<Object?> stream) { | ||
| webSocket.transform(safeJsonTransformer).changeStream((Stream<Object?> stream) { | ||
| return stream.map((Object? message) { | ||
| if (!_closed) { | ||
| _timer.reset(); |
Contributor
There was a problem hiding this comment.
Instead of creating a complex custom StreamChannelTransformer and removing the cast<String>() safety check, we can leverage Stream.handleError on the stream returned by jsonDocument. This keeps the original stream/sink pipeline intact, preserves the cast<String>() safety check, and is much more concise and idiomatic.
// Whenever we get a message, no matter which child channel it's for, we know
// the browser is still running code which means the user isn't debugging.
_channel = MultiChannel<dynamic>(
webSocket.cast<String>().transform(jsonDocument).changeStream((Stream<Object?> stream) {
return stream.handleError((Object error) {
if (error is FormatException) {
_logger.printWarning(
'Received unexpected non-JSON message from browser WebSocket: ${error.source}',
);
_logger.printTrace('JSON decode error: $error');
} else {
throw error;
}
}).map((Object? message) {
if (!_closed) {
_timer.reset();
}
return message;
});
}),
);References
- Suggest simplification and refactoring: Assess whether the code can be made simpler or refactored to enhance readability and maintainability. (link)
Contributor
Author
There was a problem hiding this comment.
Updated to use Stream.handleError with a test filter for FormatException on the jsonDocument stream.
Replaces custom transformers with Stream.handleError and explicit FormatException filters across test platform streams.
chingjun
approved these changes
Sep 4, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 5, 2026
flutter/flutter@5a6cfa7...63170e9 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338) 2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333) 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329) 2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317) 2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321) 2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316) 2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173) 2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314) 2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773) 2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780) 2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089) 2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765) 2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12767) flutter/flutter@5a6cfa7...63170e9 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338) 2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333) 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329) 2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317) 2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321) 2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316) 2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173) 2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314) 2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773) 2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780) 2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089) 2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765) 2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12767) flutter/flutter@5a6cfa7...63170e9 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 6882c3b4b542 to 77094991d1c1 (1 revision) (flutter/flutter#192339) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 10652f9d64d6 to db200e27a634 (1 revision) (flutter/flutter#192338) 2026-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from FgDQeF6jb1dVRVh3K... to _IgixhH4vEgdz9Oqn... (flutter/flutter#192333) 2026-09-05 engine-flutter-autoroll@skia.org Roll Dart SDK from 5744c2480a12 to 6882c3b4b542 (1 revision) (flutter/flutter#192331) 2026-09-05 engine-flutter-autoroll@skia.org Roll Skia from 9b1e5fd08d2c to 10652f9d64d6 (4 revisions) (flutter/flutter#192329) 2026-09-05 codefu@google.com ci(engine): target ignore_phone|none for new macs (flutter/flutter#192317) 2026-09-04 okorohelijah@google.com [devicelab] Fix Mac ios_universal_link_test CI build and scheme configuration (flutter/flutter#192321) 2026-09-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 5501d02b583d to 5744c2480a12 (5 revisions) (flutter/flutter#192316) 2026-09-04 okorohelijah@google.com [iOS] Add native deep link lifecycle integration tests for UIScene plugins (flutter/flutter#192173) 2026-09-04 engine-flutter-autoroll@skia.org Roll Skia from 93ac1e630d1d to 9b1e5fd08d2c (3 revisions) (flutter/flutter#192314) 2026-09-04 bkonyi@google.com [tool] Migrate AssembleCommand and GenerateCommand to modular dependency injection (flutter/flutter#190773) 2026-09-04 bkonyi@google.com [tool] Migrate Apple build subcommands and toolchain to modular dependency injection (flutter/flutter#190780) 2026-09-04 bkonyi@google.com [flutter_tools] Safely handle non-JSON messages in test stream parsers (flutter/flutter#192089) 2026-09-04 bkonyi@google.com [tool] Migrate LogsCommand to modular dependency injection (flutter/flutter#190765) 2026-09-04 bkonyi@google.com [tool] Migrate DevicesCommand to modular dependency injection (flutter/flutter#190768) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
bkonyi
added a commit
to flutteractionsbot/flutter
that referenced
this pull request
Sep 10, 2026
…logging When cherry-picking flutter#192089 to the stable release candidate branch `flutter-3.47-candidate.0`, `BrowserManager` in `flutter_web_platform.dart` referenced `_logger` without `_logger` being defined on `BrowserManager` (which was added on `master` in a later Chrome roll PR). This passes `_logger` from `FlutterWebPlatform` through `BrowserManager.start` and stores it on `BrowserManager` so that unexpected non-JSON messages can be logged as intended.
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
2 participants
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.
When communicating with remote test runners or browsers over test streams or WebSockets, unhandled non-JSON strings (such as dynamic library loading errors or missing snapshot warnings) caused
_ChunkedJsonParserto throw an unhandledFormatExceptionat character 1, crashing the tool.This changes
pipeHarnessToRemoteinflutter_platform.dartandBrowserManagerinflutter_web_platform.dartto intercept non-JSON payloads, log warnings and traces, and ignore invalid frames rather than crashing the tool process.Fixes #191898
Pre-launch Checklist
///).