`dagger shell` seems to be broken in v0. | Dagger | Page 1

somber wagon Dec 8, 2023, 6:09 PM

#

😵‍💫 We don't have integ tests for dagger shell yet because testing a tty command in our setup is difficult... I guess we need to figure that out asap

lethal reef Dec 8, 2023, 6:10 PM

#

testing TTY stuff in general is a bit of a quagmire so I don't blame you haha

somber wagon Dec 8, 2023, 6:14 PM

#

i'm looking into the problem, I'll see if we can add a test that at least verifies it gets passed the point it is failing at above after

#

@lethal reef oof the bug is painfully trivial too: https://github.com/dagger/dagger/pull/6240

GitHub

core: set endpoint map when initializing APIServer by sipsma · Pull...

Without this we're getting nil panics when using dagger shell.
Went unnoticed because we haven't added test coverage for dagger shell to the integ tests yet due to the fact that testing tty...

lethal reef Dec 8, 2023, 6:25 PM

#

lol, thought it might be

somber wagon Dec 8, 2023, 7:31 PM

#

fixed the v0.9.4 published engine, was just engine side so if you happened to have already tried a dagger command w/ v0.9.4 CLI then you need to run docker rmi registry.dagger.io/engine:v0.9.4, after that should be good.

dagger -m github.com/sipsma/daggerverse/yamlinvaders shell play was how I verified it worked again after that docker rmi

lethal reef Dec 8, 2023, 7:38 PM

#

nice!

somber wagon Dec 8, 2023, 10:43 PM

#

Gonna add that test now, I remembered Edgar from netflix mentioning this library he built at one point: https://github.com/Netflix/go-expect which seems very promising for this. Will involve some light shenanigans to do a go run of code that uses that library inside of one the WithExecs of our integ tests, but shouldn't be that bad hopefully. Worst case there seems to be plenty of other options available too: https://unix.stackexchange.com/questions/249723/how-to-trick-a-command-into-thinking-its-output-is-going-to-a-terminal

GitHub

GitHub - Netflix/go-expect: an expect-like golang library to automa...

an expect-like golang library to automate control of terminal or console based programs. - GitHub - Netflix/go-expect: an expect-like golang library to automate control of terminal or console based...

Unix & Linux Stack Exchange

How to trick a command into thinking its output is going to a terminal

Given a command that changes its behaviour when its output is going to a terminal (e.g. produce coloured output), how can that output be redirected in a pipeline while preserving the changed behavi...

lethal reef Dec 8, 2023, 10:55 PM

#

nice, yeah expect-like interfaces are pretty great for this sort of thing. Just keep an eye out for hangs/flakes, I remember the last few times I attempted this in Go (...or was it Ruby?) I could never get it working 100%, the same thing would always randomly hang if I ran it enough times, so I could never trust it in CI. 😭 (Even after being sure to close stdio pipes etc etc etc)

#

if you're curious/haven't seen, the name 'expect' comes from Expect (https://linux.die.net/man/1/expect / https://en.wikipedia.org/wiki/Expect) which is a pretty nifty CLI/DSL for scripting interactive programs

somber wagon Dec 8, 2023, 11:01 PM

#

lethal reef if you're curious/haven't seen, the name 'expect' comes from Expect (<https://li...

Oh yeah I have mental scars from years ago running expect from a Makefile or something stupid. It is cool but the whole problem space is just such a pain...

#

Speaking of which, on my first attempt I'm getting this panic from the CLI:

                            Caught panic:
    multi.go:85: 23: [2.34s] 
    multi.go:85: 23: [2.34s] runtime error: slice bounds out of range [3:1]
    multi.go:85: 23: [2.34s] 
    multi.go:85: 23: [2.34s] Restoring terminal...
    multi.go:85: 23: [2.34s] 
    multi.go:85: 23: [2.34s] goroutine 1 [running]:
    multi.go:85: 23: [2.34s] runtime/debug.Stack()
    multi.go:85: 23: [2.34s]    /usr/local/go/src/runtime/debug/stack.go:24 +0x64
    multi.go:85: 23: [2.34s] runtime/debug.PrintStack()
    multi.go:85: 23: [2.34s]    /usr/local/go/src/runtime/debug/stack.go:16 +0x1c
    multi.go:85: 23: [2.34s] github.com/charmbracelet/bubbletea.(*Program).Run.func1()
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/charmbracelet/bubbletea@v0.24.2/tea.go:440 +0x88
    multi.go:85: 23: [2.34s] panic({0xce0320?, 0x40002f4138?})
    multi.go:85: 23: [2.34s]    /usr/local/go/src/runtime/panic.go:920 +0x26c
    multi.go:85: 23: [2.34s] github.com/charmbracelet/bubbles/viewport.Model.visibleLines(...)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/charmbracelet/bubbles@v0.16.1/viewport/viewport.go:123
    multi.go:85: 23: [2.34s] github.com/charmbracelet/bubbles/viewport.(*Model).GotoBottom(0x40002a50c8?)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/charmbracelet/bubbles@v0.16.1/viewport/viewport.go:237 +0x154
    multi.go:85: 23: [2.34s] github.com/vito/progrock.(*Model).render(0x40002a5080)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/vito/progrock@v0.10.2-0.20230913234310-64b4a1cfb007/model.go:446 +0x1f8
    multi.go:85: 23: [2.34s] github.com/vito/progrock.(*Model).Update(0x40002a5080, {0xc723e0?, 0x400056c000?})
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/vito/progrock@v0.10.2-0.20230913234310-64b4a1cfb007/model.go:357 +0x9c
    multi.go:85: 23: [2.34s] github.com/charmbracelet/bubbletea.(*Program).eventLoop(0x40000d06c0, {0xf415e8?, 0x40002a5080?}, 0x0?)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/charmbracelet/bubbletea@v0.24.2/tea.go:373 +0x5cc
    multi.go:85: 23: [2.34s] github.com/charmbracelet/bubbletea.(*Program).Run(0x40000d06c0)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/charmbracelet/bubbletea@v0.24.2/tea.go:503 +0x6ac
    multi.go:85: 23: [2.34s] github.com/vito/progrock.(*UI).Run(0xf3d060?, {0xf46280, 0x1851a00}, 0x400025fad0?, 0x400025fb08?)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/vito/progrock@v0.10.2-0.20230913234310-64b4a1cfb007/model.go:215 +0x484
    multi.go:85: 23: [2.34s] main.inlineTUI({0xf46280, 0x1851a00}, {{0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, 0x0}, {0x400027b4a0, ...}, ...}, ...)
    multi.go:85: 23: [2.34s]    /app/cmd/dagger/engine.go:179 +0x168
    multi.go:85: 23: [2.34s] main.withEngineAndTUI({0xf46280, 0x1851a00}, {{0x0, 0x0}, {0x0, 0x0, 0x0}, {0x0, 0x0}, {0x400027b4a0, ...}, ...}, ...)
    multi.go:85: 23: [2.34s]    /app/cmd/dagger/engine.go:78 +0x15c
    multi.go:85: 23: [2.34s] main.(*FuncCommand).Command.func2(0x4000281800, {0x40003120a0, 0x1, 0x1})
    multi.go:85: 23: [2.34s]    /app/cmd/dagger/functions.go:280 +0xec
    multi.go:85: 23: [2.34s] github.com/spf13/cobra.(*Command).execute(0x4000281800, {0x40003120a0, 0x1, 0x1})
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:940 +0x658
    multi.go:85: 23: [2.34s] github.com/spf13/cobra.(*Command).ExecuteC(0x1808a00)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x320
    multi.go:85: 23: [2.34s] github.com/spf13/cobra.(*Command).Execute(...)
    multi.go:85: 23: [2.34s]    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992
    multi.go:85: 23: [2.34s] main.main()
    multi.go:85: 23: [2.34s]    /app/cmd/dagger/main.go:97 +0x34

Just in case you have any immediate ideas, I'll take a look too

lethal reef Dec 8, 2023, 11:03 PM

#

possibly need to set a larger window size on the TTY, but it definitely shouldn't panic

somber wagon Dec 8, 2023, 11:03 PM

#

lethal reef possibly need to set a larger window size on the TTY, but it definitely shouldn'...

Oh cool, I'll see if that's possible, thanks!

lethal reef Dec 8, 2023, 11:05 PM

#

I see expect exposes the Tty as an *os.File, so this should do the trick: https://pkg.go.dev/github.com/creack/pty#Setsize (I use this package in midterm/progrock)

somber wagon Dec 8, 2023, 11:06 PM

#

Ah thanks 😄 I was just gonna say I need to go search for the right ioctl incantation, that's much easier

#

that worked!

#

Moving onto a Error: websocket: bad handshake... Which is probably due to the fact it's running inside a nested exec. I'm guessing the shim doesn't proxy it right... At least that's A) incredibly obscure that it's not a show stopper bug and B) not a tty problem laughcry

#

Hopefully easy to fix

somber wagon Dec 9, 2023, 4:12 AM

#

^ famous last words
But fortunately I figured out the prereqs to even get dagger shell working in a nested context, which ended up being some fixes to send the auth headers and then handling websockets in engine/client.ServeHTTP by hijacking the connection and proxying that.

I also along the way found out we are leaking goroutines in the shell handler because we weren’t closing the stdio pipes in service.Start 🥲

And then at the end of it all while shell finally worked i couldn’t get the expect lib to work with expecting the shell prompt. Unknown why but I can get it to work by writing input lines directly and just using sleep instead of waiting for prompts, so i’ll probably just try to use something simpler at that point…

#

Either way, friday’d out so ill just finish up later

lethal reef Dec 9, 2023, 5:52 AM

#

Hmm... Maybe it really doesn't like the constant printing in the TUI? In hindsight, I've only used expect with basic interactive apps, not full-blown TUIs. Maybe we need a simplified mode for testing, sort of like --progress=plain

#`dagger shell` seems to be broken in v0.