nforgeio / neonKUBE

Public NeonKUBE Kubernetes distribution related projects
https://neonkube.io
Apache License 2.0
76 stars 13 forks source link

[neon-cli]: ArgumentOutOfRangeException in Windows Terminal #1890

Closed jefflill closed 3 months ago

jefflill commented 3 months ago
╭─ Marcus Bowyer@JUPITER · C:\src\neonCLOUD · master ±0 ?1                                                                                                                                                 10s
╰─❯ neon cluster delete

Are you sure you want to delete cluster: marcus-aws-small? [y/n]: y

Deleting: marcus-aws-small...
Deleted:  marcus-aws-small

 Setup [marcus-aws-small] cluster

 Steps: [---------------------------->  ]

     17. persist state           [x] DONE
     18. node labels (cloud)     [x] DONE
     19. ssh block ingress       [x] DONE

 Control-Plane:
    control-0       READY
    control-1       READY
    control-2       READY

 Workers:
    worker-0        READY
    worker-1        READY
    worker-2        READY

 Step Runtime
 ------------
 resource requirements:    00:00:00.0341500 (0.03415 sec)
 connect aws:              00:00:02.6438202 (2.6438202 sec)
 ssh port mappings:        00:00:08.4574122 (8.4574122 sec)
 connect nodes:            00:00:08.2291472 (8.2291472 sec)
 check node OS:            00:00:00.0019940 (0.001994 sec)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 29.
   at System.ConsolePal.SetCursorPosition(Int32 left, Int32 top)
   at Neon.Kube.Setup.SetupConsoleWriter.Update(String text) in C:\src\neonKUBE\Lib\Neon.Kube\SetupController\SetupConsoleWriter.cs:line 144
   at Neon.Kube.Setup.SetupClusterStatus.WriteToConsole(Int32 maxDisplayedSteps, Boolean showNodeStatus, Boolean showRuntime) in C:\src\neonKUBE\Lib\Neon.Kube\SetupController\SetupClusterStatus.Console.cs:line 400
   at NeonCli.ClusterSetupCommand.<>c.<RunAsync>b__8_1(SetupClusterStatus status) in C:\src\neonKUBE\Tools\neon-cli\Commands\Cluster\ClusterSetupCommand.cs:line 306
   at Neon.Kube.Setup.SetupController`1.<>c__DisplayClass119_0.<RunAsync>b__0() in C:\src\neonKUBE\Lib\Neon.Kube\SetupController\SetupController.cs:line 1694

╭─ Marcus Bowyer@JUPITER · C:\src\neonCLOUD · release-neonkube-v0.11.0-beta.0 ±0 ?1                                                                                                        0xe0434352 · 29m 5s
╰─❯ cd ..
jefflill commented 3 months ago

We're probably trying to write past the height of the screen buffer.

Try increasing the height to something big like 1K and then adding a check to prevent us from trying to write past the end.

jefflill commented 3 months ago

I just noticed that @marcusbooyah was running this in a normal Windows CMD shell. I normally user Cmdr, so perhaps that's the difference.

jefflill commented 3 months ago

So @marcusbooyah is using Windows Terminal. I haven't been able to replicate this with Cmdr or cmd.exe yet and I also haven't been able to replicate this in Windows Terminal when deploying a single-node cluster.

I'll have to try again with a larger cluster which will result in more lines being displayed on the console.

jefflill commented 3 months ago

I've been able to replicate this in Terminal: the trick is to resize the terminal to be smaller than what the console output will be.

This doesn't seem to be a problem with Cmder and cmd.exe.

Possibly related:

https://github.com/microsoft/terminal/discussions/14975#discussioncomment-5258673 https://github.com/microsoft/terminal/issues/7434 (detect Windows Terminal?) https://github.com/PowerShell/PSReadLine/issues/1155 https://github.com/PowerShell/PSReadLine/pull/1182

This confirms that Windows Terminal is different: https://stackoverflow.com/questions/75250559/console-application-window-and-buffer-sizes-in-windows-11

Console Screen Buffer Docs:

https://learn.microsoft.com/en-us/windows/console/console-screen-buffers (alternate buffer mode?)

jefflill commented 3 months ago

Hmmm: Windows Terminal seems to be overriding my Console.SetBufferSize() after I manually resize the terminal window.

I think the workaround may be to have SetupConsoleWriter.Update() compare the current buffer size to the expected size and when they differ\:

  1. reset the buffer size
  2. clear the screen
  3. clear our internal buffer

...before updating the screen (which will be a full update since we cleared our buffer).

UPDATE: This didn't work; we still see ArgumentOutOfRangeExceptions.

It also looks like MSFT fundamentally disagrees with allowing users to set a larger buffer than the screen size:

https://github.com/microsoft/terminal/issues/6895#issuecomment-658997728 https://github.com/microsoft/terminal/issues/1860

(funny) I saw one of the maintainers say that there's no real use case for this and also that no terminal window in the history of the world does this. The immediate response was that wrapping wide table output is a mess and that cmd.exe has always allowed this.

jefflill commented 3 months ago

HACKED: I ended up hacking around this by using the existence of the WT_SESSION environment variable (which is not recommended) to determine whether we're running in Windows Terminal and we disable the fancy double buffer in this case.

This doesn't crash but the user experience isn't great. Setting aside the additional flashing (which is isn't nearly as bad in Terminal), Terminal is going to do crazy line wrapping when the window isn't wide enough and users won't really be able to scroll vertically because we're going to be continually clearing and updating the screen.

MSFT recommends using ANSI TERM escape sequences instead of the System.Console API and we should look into that in the future, but I'm not at all certain that will help in this situation because it looks like Terminal just doesn't support buffers wider than the screen.

We will need to support ANSI TERM escape sequences eventually because the System.Console API doesn't support MacOS or Linux.