Bug Report: Cannot find profile --> Crash #2455
Labels
Area-Interaction
Interacting with the vintage console window (as opposed to driving via API or hooks)
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
Priority-1
A description (P1)
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Severity-Crash
Crashes are real bad news.
Milestone
Comments
carlos-zamora
added
Area-Interaction
ghost
added
the
Needs-Triage
carlos-zamora
removed
the
Needs-Triage
|
for the record, I don't think this was introduced in #2422, this probably dates back to pre-open source, when we started falling back to the default profiles if the settings were invalid. This just makes it more apparent, because we crash less aggressively. |
zadjii-msft
added a commit
that referenced
this issue
Sep 13, 2019
Catch all calls to FindProfile and MakeSettings, which might fail if the profile doesn't exist
zadjii-msft
added a commit
that referenced
this issue
Sep 13, 2019
This is work that I was doing during #2455 to try and get the TabTests working again. It doesn't work, as much as I'd like them to. I have mail out to try and get them working but I think we're stumped right now.
Closed
4 tasks
DHowett-MSFT
pushed a commit
that referenced
this issue
Mar 26, 2020
This PR has evolved to encapsulate two related fixes that I can't really untie anymore. #2455 - Duplicating a tab that doesn't exist anymore This was the bug I was originally fixing in #4429. When the user tries to `duplicateTab` with a profile that doesn't exist anymore (like might happen after a settings reload), don't crash. As I was going about adding tests for this, got blocked by the fact that the Terminal couldn't open _any_ panes while the `TerminalPage` was size 0x0. This had two theoretical solutions: * Fake the `TerminalPage` into thinking it had a real size in the test - probably possible, though I'm unsure how it would work in practice. * Change `Pane`s to not require an `ActualWidth`, `ActualHeight` on initialization. Fortuately, the second option was something else that was already on my backlog of bugs. #4618 - `wt` command-line can't consistently parse more than one arg Presently, the Terminal just arbitrarily dispatches a bunch of handlers to try and handle all the commands provided on the commandline. That's lead to a bunch of reports that not all the commands will always get executed, nor will they all get executed in the same order. This PR also changes the `TerminalPage` to be able to dispatch all the commands sequentially, all at once in the startup. No longer will there be a hot second where the commands seem to execute themselves in from of the user - they'll all happen behind the scenes on startup. This involved a couple other changes areound the `TerminalPage` * I had to make sure that panes could be opened at a 0x0 size. Now they use a star sizing based off the percentage of the parent they're supposed to consume, so that when the parent _does_ get laid out, they'll take the appropriate size of that parent. * I had to do some math ahead of time to try and calculate what a `SplitState::Automatic` would be evaluated as, despite the fact that we don't actually know how big the pane will be. * I had to ensure that `focus-tab` commands appropriately mark a single tab as focused while we're in startup, without roundtripping to the Dispatcher thread and back ## References #4429 - the original PR for #2455 #5047 - a follow-up task from discussion in #4429 #4953 - a PR for making panes use star sizing, which was immensly helpful for this PR. ## Detailed Description of the Pull Request / Additional comments `CascadiaSettings::BuildSettings` can throw if the GUID doesn't exist. This wraps those calls up with a try/catch. It also adds a couple tests - a few `SettingsTests` for try/catching this state. It also adds a XAML-y test in `TabTests` that creates a `TerminalPage` and then performs som UI-like actions on it. This test required a minor change to how we generate the new tab dropdown - in the tests, `Application::Current()` is _not_ a `TerminalApp::App`, so it doesn't have a `Logic()` to query. So wrap that in a try/catch as well. While working on these tests, I found that we'd crash pretty agressively for mysterious reasons if the TestHostApp became focused while the test was running. This was due to a call in `TSFInputControl::NotifyFocusEnter` that would callback to `TSFInputControl::_layoutRequested`, which would crash on setting the `MaxSize` of the canvas to a negative value. This PR includes a hotfix for that bug as well. ## Validation Steps Performed * Manual testing with a _lot_ of commands in a commandline * run the tests * Team tested in selfhost Closes #2455 Closes #4618
ghost
added
Resolution-Fix-Committed
|
🎉This issue was addressed in #5090, which has now been successfully released as Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
Introduced by #2422
Since we'll be defaulting to the Windows Terminal default settings, this means that the user may be working with profiles that don't exist AFTER the proper settings are imported. Below is an example of such a case, but several other places may cause the same issue to arise.
I'd bet that this is because we usually use "FindProfile()" (or something like that) and we just don't handle that properly.
Steps to reproduce
Expected behavior
Not sure. Maybe a dialogue box saying "This operation could not be completed"?
Actual behavior
CRASH!
The text was updated successfully, but these errors were encountered: