Skip to content

Handle debugger restart request (with a cold restart) for Flutter profile/release mode #4281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DanTup opened this issue Nov 23, 2022 · 3 comments · Fixed by flutter/flutter#121610
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps is enhancement relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Nov 23, 2022

See #4276 (comment).


We can't hide the "restart" button (which we usually use for Hot Restart) because VS Code doesn't allow it (restart should always be possible, even if it's not a Hot Restart, because it should just be the same as stopping the session and starting it again). We also can't currently tell VS Code to handle the restart because we only know about profile/release flags in launchRequest (not during startup when capabilities are exchanged).

So there are a few options:

  1. Do nothing. This means the restart button on the debug toolbar will not work, and instead prints an error to the console in profile/release mode:

    Failed to Hot Restart: DebugAdapterException: Exception: Restart is not supported in profile mode

  2. Handle restartRequest in the DAP and terminate the flutter run process, clean up all state, and then restart it (this is probably the best solution, but also complex)
  3. Pass an additional flag for profile/release mode to the flutter debug_adapter process so it's known before launchRequest, and use that to set the supportsRestartRequest capability.

1 is ofc easiest, but I think we should probably do 2.

(on a related note, we should ensure Hot Restart commands are disabled/hidden from the palette etc., since we're just implementing a standard restart here, and having Hot Restart commands show up may be confusing where it's not supporting and we will do a cold restart).

@DanTup DanTup added is enhancement in flutter Relates to running Flutter apps in debugger Relates to the debug adapter or process of launching a debug session labels Nov 23, 2022
@DanTup DanTup added this to the v3.54.0 milestone Nov 23, 2022
@DanTup DanTup modified the milestones: v3.54.0, v3.56.0 Nov 28, 2022
@DanTup DanTup modified the milestones: v3.56.0, On Deck Dec 12, 2022
@DanTup DanTup modified the milestones: On Deck, v3.62.0 Feb 28, 2023
@DanTup
Copy link
Member Author

DanTup commented Feb 28, 2023

We can use CapabilitiesEvent to advertise support for supportsRestartRequest later.

DanTup added a commit to DanTup/flutter that referenced this issue Feb 28, 2023
Before this change, the Flutter adapter always sent "supportsRestartRequest: true" to the DAP client. This told the DAP client if the user clicks the "Restart" button on the debug toolbar that we (the DAP server) will handle the restart (allowing us to Hot Restart instead of a slower termination and restart of the entire debug session).

However, Profile/Release mode do not support Hot Restart, so this would result in an error "Hot Restart failed".

This change no longer sets `supportsRestartRequest: true` at startup, but instead waits for the `app.start` event, and sends the value of its `supportsRestart` field (an existing field that was already available in the `app.start` event).

This results in Hot Restart being available for debug builds, but the DAP client/IDE performing a full debug session restart for other modes.

Fixes Dart-Code/Dart-Code#4281.
@DanTup
Copy link
Member Author

DanTup commented Feb 28, 2023

flutter/flutter#121610

@DanTup
Copy link
Member Author

DanTup commented Mar 6, 2023

Fixed by flutter/flutter#121610.

@DanTup DanTup closed this as completed Mar 6, 2023
@DanTup DanTup modified the milestones: v3.62.0, Requires SDK Release Mar 6, 2023
@DanTup DanTup modified the milestones: Requires SDK Release, v3.64.0 May 2, 2023
@DanTup DanTup added the relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available label May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps is enhancement relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant