Skip to content
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

CRASH: ConsoleManager throws ArgumentOutOfRangeException while using the REPL interactively #443

Open
fredrikhr opened this issue Nov 16, 2020 · 20 comments
Labels
bug Something isn't working triage

Comments

@fredrikhr
Copy link

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 the ConsoleManager class, Method: MoveCaret(Int32 position).

...> get someapi/somepath/someendpoi
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)

dotnet --info

.NET SDK (reflecting any global.json):
 Version:   5.0.100
 Commit:    5044b93829

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19042
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100\

Host (useful for support):
  Version: 5.0.0
  Commit:  cf258a14b7

.NET SDKs installed:
  3.1.402 [C:\Program Files\dotnet\sdk]
  5.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

dotnet tool list --global

Package Id                            Version                  Commands
------------------------------------------------------------------------------------------
microsoft.dotnet-httprepl             5.0.0                    httprepl
@tlmii tlmii added the bug Something isn't working label Nov 16, 2020
@tlmii
Copy link
Member

tlmii commented Nov 16, 2020

@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 tlmii added the triage label Nov 16, 2020
@fredrikhr
Copy link
Author

@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.

@fredrikhr
Copy link
Author

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)

@tlmii
Copy link
Member

tlmii commented Nov 16, 2020

@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!

@tlmii
Copy link
Member

tlmii commented Nov 16, 2020

Yep, I can reproduce this now reliably as well. Something must have regressed. I'll see what I can figure out.

@tlmii
Copy link
Member

tlmii commented Nov 17, 2020

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:

  • cmd.exe works fine,
  • wt.exe crashes and
  • vsdebugconsole.exe has an issue with duplicating the last character from the previous line and putting the cursor in the wrong place (but it doesn't crash).

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.

@tlmii
Copy link
Member

tlmii commented Nov 17, 2020

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.

  1. There's an assumption in the ConsoleManager/InputManager that the caret position in the host (cmd, powershell, whatever) will always move "forward" (to the right and down) when Console.Write(string) is called. But this isn't necessarily true. Specifically, in some hosts the caret won't move when you type a character in the last position on the row. It doesn't move until you add the next character (the one that would be the first one on the next line). This leads to several problems where the tool thinks the cursor is somewhere within the line rather than at the end of the line. Depending on what your next action is - typing a character, moving the cursor, etc - the exception can vary.
  2. There's another assumption that when the line wrapping occurs, the Console.CursorTop will move to the next line. This is true if you're not at the end of the buffer. But if you're at the end of the buffer, everything is shifted up and you stay on the same line. This throws off the calculations for the current caret position. This problem manifests itself differently between hosts because of the varying size (or definitions) of the buffers. For example: cmd.exe running by itself has a large buffer height (9001) and so it takes a long time for you to run into this. But cmd.exe running within Windows Terminal has a buffer height of the size of the window, so you run into it more quickly.

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:

private IDisposable CaretUpdateScope()
{
Point currentCaret = Caret;
return new Disposable(() =>
{
Point c = Caret;
int y = c.Y - currentCaret.Y;
int x = c.X - currentCaret.X;
CaretPosition += y * Console.BufferWidth + x;
});
}

Caret in this case is just a wrapper around Console.CursorLeft and Console.CursorTop:

public Point Caret => new Point(Console.CursorLeft, 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.

@bradygaster
Copy link
Member

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?

@tlmii
Copy link
Member

tlmii commented Nov 17, 2020

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)

HttpReplImpropertInsertionIssue

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)

HttpReplCrashIssue

@bradygaster
Copy link
Member

In which terminal did you do those, @tlmii?

@tlmii
Copy link
Member

tlmii commented Nov 18, 2020

Windows Terminal

@tlmii
Copy link
Member

tlmii commented Dec 11, 2020

@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!

@tlmii
Copy link
Member

tlmii commented Dec 21, 2020

@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.

@Tragetaschen
Copy link

I'm still able to produce the SetCursorPosition AOORException with 5.0.1.

My reproduction steps in Windows Terminal are (see Gif)

  • Make sure the prompt is at the bottom of the window—I could not reproduce without that
  • Start httprepl
  • Connect to a long URI (I used an Azure Logic App) that wraps around the prompt
  • Edit a new command (I want to set VS Code as my editor)
  • After pasting parts of the VS Code path, I move the cursor to the beginning and type
    • C (nothing interesting happens)
    • : (stuff gets duplicated)
    • \ (more stuff gets duplicated)
    • <End> Crash

httprepl-aoore

(I also recreated the Access Key after recording the Gif if anyone wonders…)

@tlmii
Copy link
Member

tlmii commented Jan 7, 2021

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.

@tlmii
Copy link
Member

tlmii commented Jan 7, 2021

@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.

@Tragetaschen
Copy link

Tragetaschen commented Jan 7, 2021

You probably saw this already: In 5.0.2-preview.21056.2 when you delete a character, the cursor always jumps to the end.

@tlmii
Copy link
Member

tlmii commented Jan 7, 2021

@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

@tlmii
Copy link
Member

tlmii commented Jan 8, 2021

@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

@Tragetaschen
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage
Projects
None yet
Development

No branches or pull requests

4 participants