Open fredrikhr opened 4 years ago
@fredrikhr This is interesting. I've seen this before, but never consistently like you're describing. Since you're seeing it regularly, maybe we can figure out the root cause. Your description is already helpful. Let me know if you notice anything else that narrows it down.
Out of curiosity, are you using a non-English keyboard layout? Just wondering if something connected to the bad handling of things like AltGr (see #226 as an example) might be in play here as well.
@tlmii Hmm, I am using a non-English keyboard (Norwegian Bokmål) in my case, but I am not using AltGr in today' crash.
Related to the issue described I also noticed a problem when pasting a long-string into the terminal window (Right-click->paste or Shift+Insert) so that the inserted string goes beyond the current width of the window. The error message is slightly different in that case:
Unhandled exception. System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)
at Microsoft.Repl.Input.InputManager.FlushInput(IShellState state, List`1& presses) in /_/src/Microsoft.Repl/Input/InputManager.cs:line 380
at Microsoft.Repl.Input.InputManager.StartAsync(IShellState state, CancellationToken cancellationToken) in /_/src/Microsoft.Repl/Input/InputManager.cs:line 318
at Microsoft.HttpRepl.Program.Start(String[] args, IConsoleManager consoleManager, IPreferences preferences, ITelemetry telemetry) in /_/src/Microsoft.HttpRepl/Program.cs:line 96
at Microsoft.HttpRepl.Program.Main(String[] args) in /_/src/Microsoft.HttpRepl/Program.cs:line 27
at Microsoft.HttpRepl.Program.<Main>(String[] args)
The problem also occurs when pressing the Up and Down arrows keys for command-replay when the command-to-replay is longer than the current width of the window.
I can also confirm that I observe the same behaviour both on Windows and on Ubuntu (running in WSL2). Also running from within Windows Terminal or directly from cmd.exe does not make any difference.
Another exception message, this time when editing a previously run command (by hitting up-arrow), again exception was throws as soon as the caret moved one beyond the current width of the window:
Unhandled exception. System.ArgumentOutOfRangeException: The value must be greater than or equal to zero and less than the console's buffer size in that dimension. (Parameter 'top')
Actual value was 49.
at System.ConsolePal.SetCursorPosition(Int32 left, Int32 top)
at Microsoft.Repl.ConsoleHandling.ConsoleManager.MoveCaret(Int32 positions) in /_/src/Microsoft.Repl/ConsoleHandling/ConsoleManager.cs:line 125
at Microsoft.Repl.Input.InputManager.StartAsync(IShellState state, CancellationToken cancellationToken) in /_/src/Microsoft.Repl/Input/InputManager.cs:line 339
at Microsoft.HttpRepl.Program.Start(String[] args, IConsoleManager consoleManager, IPreferences preferences, ITelemetry telemetry) in /_/src/Microsoft.HttpRepl/Program.cs:line 96
at Microsoft.HttpRepl.Program.Main(String[] args) in /_/src/Microsoft.HttpRepl/Program.cs:line 27
at Microsoft.HttpRepl.Program.<Main>(String[] args)
@fredrikhr this is great info. I'll dig in today and see if I can find out what's going on. Thanks for the details!
Yep, I can reproduce this now reliably as well. Something must have regressed. I'll see what I can figure out.
Well, there's definitely a bug with the end of line/line wrap parsing. The "fun" part is the behavior appears to be different depending on which terminal its run in - even just on windows. cmd.exe, vsdebugconsole.exe and wt.exe all act differently. For me:
I'm guessing I'd see variants of those three when running on MacOS and linux as well.
Though its interesting that you said the problem occurred for you in cmd.exe as well. I wonder if there's some sort of configuration of the terminal that could impact this.
For the record, I'm able to reproduce this just by typing 0123456789 over and over until it wraps. So I'm not sure how this wasn't caught before, though the console handling code didn't get as much of a deep dive as the rest when we took it over.
OK, so here's what I've been able to determine. There's (at least) two different issues going on here, both with the same (or related) root causes.
It seems that the tie between the host's cursor position and the ConsoleManager's cursor position (which is basically just an index on an array, not a 2D position like the host) is not as clear as it was thought to be originally. Specifically, this method, which uses before/after snapshots of the host's positions to update the tool's position whenever a change or set of changes are made:
Caret
in this case is just a wrapper around Console.CursorLeft
and Console.CursorTop
:
There's another piece that's definitely off here as well: We are updating the cursor position with these methods even in the case of things being written to the screen as part of command execution (e.g. error messages, response content, etc). It'll never be right in those cases because of all the escape codes contained in the content. But we reset it every time at the end of a command execution anyway. So its unnecessarily and incorrectly tracked... and then reset.
We could - maybe - split the way that is implemented between user input and system output, and then use the length of what the user entered to calculate the tool's caret position rather than using the host's position. Or maybe we could rely on the reset mentioned in the last paragraph to not have to do a split and just do it that way regardless.
There might be issues with supporting other languages/keyboards if we use the string length... but I think those would already be an issue with the current implementation (disconnect between the number of chars in a .NET string and the number of characters printed to the screen in a console).
Going to have to ponder this, but wanted to put all the info out there in case anyone else had any thoughts.
This is really great info. I tried a few things last night on my mac and couldn't reproduce this but have a few more tests I need to do today. I will use this as a guide to testing this out to see if I can reproduce it.
I know this is a stretch to request, but do you happen to have a video of this behavior?
Here's a couple of screen recording gifs that show the two issues:
First, the improper insertion point issue related to the host's cursor position not moving forward when you type the last character on a line (the 3 is the last character typed, then you see the 4 is inserted before it with the 3 pushed to the next line, and the 5 then inserted between them)
Second, the crash that occurs when you're also at the end of the buffer (the 3 is added to the end of the line, the crash occurs when I press 4)
In which terminal did you do those, @tlmii?
Windows Terminal
@fredrikhr We've released a preview build with an attempt at a fix for this. It should be available on NuGet.
You can upgrade by running:
dotnet tool update --global microsoft.dotnet-HttpRepl --version 5.0.1-preview.20611.1
Note that you may not see the version right away - it might take an hour or two to be available in your region.
Can you give the build a chance and see if it fixes the issues you were seeing? I also want to make sure we didn't break anything else - it was a relatively foundational change, so let me know if you see any other oddities with cursor position and input while testing.
And thanks again for reporting this and giving us details!
@fredrikhr just checking to see if you had a chance to try out the latest preview and see if it worked for you. My testing so far has gone well, but I'd like to see a little more feedback before pushing 5.0.1 to release.
I'm still able to produce the SetCursorPosition
AOORException with 5.0.1.
My reproduction steps in Windows Terminal are (see Gif)
httprepl
C
(nothing interesting happens):
(stuff gets duplicated)\
(more stuff gets duplicated)<End>
Crash(I also recreated the Access Key after recording the Gif if anyone wonders…)
Thanks @Tragetaschen for the detailed steps. I knew there'd be some more cases missed from the main fix, this will hopefully help me track them down. I'll dig in and see if I can identify this issue.
@Tragetaschen Can you try the new 5.0.2 preview?
dotnet tool update microsoft.dotnet-Httprepl --version 5.0.2-preview.21056.2 --global
It should act better... but I'd like to see if it completely fixes the issue for you.
Note that it might not be available for you right away as I just published it.
Edit: On further inspection I can see there's a few more problems than the one I identified in this fix. I'll see about getting a more complete fix out later today.
You probably saw this already: In 5.0.2-preview.21056.2
when you delete a character, the cursor always jumps to the end.
@Tragetaschen
You probably saw this already: In
5.0.2-preview.21056.2
when you delete a character, the cursor always jumps to the end.
Yeah, that's what I was referring to. It was in 5.0.1 as well. I'll get another preview fix out today
@Tragetaschen Got a new build for testing: 5.0.2-preview.21057.1. Same caveat as the last one about it take a bit for it to show up on nuget feeds.
dotnet tool update microsoft.dotnet-httprepl --version 5.0.2-preview.21057.1 --global
There is still some weirdness, though I was not able to reproduce any AOORE. I'll create a new issue to not hijack this one too much
I regularly encounter that the HTTP REPL suddendly crashes while I am typing commands in the REPL. From the error message below I suspect this happens when I type beyond the width of my terminal window. I also suspect that this happens when I have edited the command along the way (i.e. corrected the typed command with pressing backspace, navigating back and forth with the arrow-keys, etc.)
The REPL throws an
ArgumentOutOfRangeException
from theConsoleManager
class, Method:MoveCaret(Int32 position)
.dotnet --info
dotnet tool list --global