Repository navigation
[flutter_tools] Handle all DartDevelopmentServiceExceptions gracefully in resident web runner - #191183
Conversation
…y in resident web runner When DDS fails to establish a WebSocket connection or upgrade the connection to DWDS (e.g. if the target browser exits prematurely or rejects the handshake), package:dds throws DartDevelopmentServiceException with errorCode connectionError rather than failedToStartError. ResidentWebRunner previously only caught failedToStartError and rethrew all other DartDevelopmentServiceExceptions, which caused the CLI to crash with an unhandled top-level exception. This change catches all DartDevelopmentServiceExceptions in ResidentWebRunner.run, logs the error, and gracefully exits via throwToolExit(kExitMessage).
github-actions
Bot
added
the
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies ResidentWebRunner to catch DartDevelopmentServiceException during startup, log the error and stack trace, and throw a tool exit. It also adds a unit test to verify that the runner does not crash when DDS fails to upgrade the WebSocket. Feedback points out that logging all DartDevelopmentServiceExceptions introduces a regression by printing noisy stack traces for expected startup failures (specifically failedToStartError). It is recommended to preserve the quiet behavior for expected failures and use error.message instead of error.toString().
| } on DartDevelopmentServiceException catch (error, stackTrace) { | ||
| // The application may have started shutting down before DDS was able to finish establishing | ||
| // its connection to DWDS. Don't treat this as an unhandled exception. | ||
| appFailedToStart(); | ||
| if (error.errorCode == DartDevelopmentServiceException.failedToStartError) { | ||
| throwToolExit(kExitMessage); | ||
| } | ||
| rethrow; | ||
| _logger.printError(error.toString(), stackTrace: stackTrace); | ||
| throwToolExit(kExitMessage); |
There was a problem hiding this comment.
Previously, when error.errorCode == DartDevelopmentServiceException.failedToStartError, the runner would call throwToolExit(kExitMessage) directly without printing any error message or stack trace to the console. This is because failedToStartError is an expected failure mode where additional stack traces are unnecessary and noisy for the user.
With the current changes, all DartDevelopmentServiceExceptions (including failedToStartError) will print the error and stack trace via _logger.printError, which introduces a regression in the user experience by printing noisy stack traces for expected startup failures.
We should preserve the quiet behavior for failedToStartError and only log the error/stack trace for other unexpected DartDevelopmentServiceException error codes. Also, using error.message is cleaner than error.toString() as it avoids the class name prefix.
| } on DartDevelopmentServiceException catch (error, stackTrace) { | |
| // The application may have started shutting down before DDS was able to finish establishing | |
| // its connection to DWDS. Don't treat this as an unhandled exception. | |
| appFailedToStart(); | |
| if (error.errorCode == DartDevelopmentServiceException.failedToStartError) { | |
| throwToolExit(kExitMessage); | |
| } | |
| rethrow; | |
| _logger.printError(error.toString(), stackTrace: stackTrace); | |
| throwToolExit(kExitMessage); | |
| } on DartDevelopmentServiceException catch (error, stackTrace) { | |
| // The application may have started shutting down before DDS was able to finish establishing | |
| // its connection to DWDS. Don't treat this as an unhandled exception. | |
| appFailedToStart(); | |
| if (error.errorCode != DartDevelopmentServiceException.failedToStartError) { | |
| _logger.printError(error.message, stackTrace: stackTrace); | |
| } | |
| throwToolExit(kExitMessage); |
There was a problem hiding this comment.
Updated in commit f86ca0c to preserve quiet behavior for failedToStartError and log error.message with stack trace for unexpected error codes.
flutter-dashboard
Bot
removed
the
autosubmit
bkonyi
added
cp: stable
Description
This PR ensures that
ResidentWebRunnercatches all variants ofDartDevelopmentServiceExceptionduring startup, rather than justfailedToStartError.When DDS fails to establish a WebSocket connection or upgrade the connection to DWDS (e.g., if the target browser exits prematurely or rejects the handshake),
package:ddsthrowsDartDevelopmentServiceException.connectionIssue(...)witherrorCode = DartDevelopmentServiceException.connectionError(Code 3), rather thanfailedToStartError(Code 2).Previously,
ResidentWebRunneronly caughtfailedToStartErrorand rethrew all otherDartDevelopmentServiceExceptions, which caused the CLI to crash with an unhandled top-level exception.With this change, we catch all
DartDevelopmentServiceExceptions, log the error details with stack trace, perform necessary cleanup, and gracefully exit viathrowToolExit(kExitMessage).Related Issues
Fixes #191179
Tests
Added a new unit test in
resident_web_runner_test.dart:Does not crash if DDS fails to upgrade WebSocket during startup