vhs
vhs copied to clipboard
charmbracelet
feat: add Wait to wait for expected output
This PR adds two new commands:
MatchLinethat takes a regular expression and waits until the current line matchesMatchScreenthat takes a regular expression and waits until the "screen" matches
The purpose is to allow waiting for things like network or serial devices that might have more variable delays while using VHS.
MatchLine example
Output "out.gif"
Type "sudo apt update" Enter
MatchLine "password"
Type "hunter2" Enter # fake password :D
MatchLine "^>"
Type "sudo apt dist-upgrade" Enter
MatchLine "^>"
Sleep 1
MatchScreen example
Output "out.gif"
Set Width 1280
Set Height 720
Type "telnet telehack.com" Enter
MatchScreen "command list"
Type "starwars" Enter
MatchScreen "A long time ago"
Ctrl+C
MatchLine "^\."
Type "exit" Enter
MatchLine "^>"
Sleep 1
Additional Info
- A few
string(s[0]) + strings.ToLower(s[1:])places were changed to atoCamelhelper with unit test (fixes issue withMUTLI_WORDcommands formatting) - A
.Buffer()method was added to*VHSto allow getting the current buffer as text (screen) - A
.CurrentLine()method was added to*VHSto allow getting the current line as text .SaveOutput()was updated to use the shared method (largely the same code as before)- Possibly closes #70
I've been experimenting with this and it's a useful feature, thanks for creating this!
I've noticed though that it would benefit from a timeout option in the event the expected pattern isn't encountered?
@rbergmanaf Yeah, I could see using that; should it be optional or required? If optional, should it have a default? It seems odd to have something hang forever by default. Something short, like 3 seconds, and then you can manually raise it for long commands.
If the default is shorter, then it's more likely a user will put their value in for longer-running commands -- the last thing we want is a script that takes a long time failing halfway through because the user didn't realize there was a 30-second timeout or something.
MatchLine <regex> [timeout]
I considered using @ like Type (e.g., MatchLine@1s), but it doesn't read well.
Alternatively, since it is a waiting command, we could just make it mandatory.
I don't feel strongly, but do like the idea of a short default. @ would probably be the least surprising given the precedent in Type, but as long as it's documented I imagine either way being acceptable personally. Maybe the charm team would have a preference?
I agree on the time out being a great option, I agree on the default of 3 seconds as well having the user manually bump it if they know it will be longer.
Additionally, I would probably have this renamed to Wait (possibly WaitFor / WaitUntil) so it reads a bit like english commands:
Type 'echo "Hello"'
Enter
Wait /hello/ 5s
A simplified command like Wait that just waits for the next prompt to appear without needing an explicit expression for that one case does seem really convenient. It's the main way we are using it at this point, though we have some other cases that benefit from also having the ability to customize the regex.
I like how Wait reads as well as implies what it does -- waits for a thing. Follow the what rather than the how.
I think WaitForLine is pretty clear, not sure if WaitForScreen reads as well, though. What about WaitOnScreen?
As for the common case of waiting for prompts, we could potentially do a Wait variation that uses WaitForLine but with a Set-able PROMPT variable or something like that with a sane default. Line & Screen could require a pattern, and Wait would not accept one to prevent the overlap.
I would propose something like the following:
Wait[+Context][@timeout] [regex]
Wait for prompt
Wait # simply wait for the prompt to appear (default timeout of 5 seconds)
Wait with timeout.
Wait@3m # wait for a maximum of 3 minutes for the prompt to appear
Wait with regex, we can decided whether the default is a screen or line.
Wait /hello/ # wait for hello to appear
Wait with context
Wait+Screen /hello/ # wait for hello to appear on the screen.
Wait+Line /hello/ # wait for hello to appear on the line.
Wait with all options:
Wait+Screen@1m /hello/
What are your thoughts? I think this syntax is nicer since it limits the commands but I'm very open to having different API that is potentially nicer?
Context is a nice idea, is that used elsewhere or would this be a new syntax?
As for defaulting, I think we should set the defaults for our "wait for prompt" use case.
Set WaitPattern />$/ # default wait pattern
Set WaitTimeout 5s # default wait timeout
Wait # can be used in most cases to wait for the next prompt
Wait+Line # identical to the above
Wait@1m # for longer-running commands
Wait /custom-pattern/
Wait+Screen /dialog title/ # match anywhere on the screen
As for defaulting, I think we should set the defaults for our "wait for prompt" use case.
Yup, sounds good!
Context is a nice idea, is that used elsewhere or would this be a new syntax?
This + syntax is used with the Ctrl and Alt commands, i.e.:
Ctrl+L
Alt+Enter
In terms of implementation, the Command would be Wait and the context would be the Options.
Okay, that all sounds reasonable, and thanks for the insight on context.
The last bit to clarify is regex parsing -- we can probably piggyback on readString and pass / as the endChar with a new token type REGEX to keep things clear.
Does all sound good?
Excellent, I'll switch this PR to a draft until I'm able to get those changes pushed up for review
Is this PR still planned to be merged at some point? I would benefit a lot from the capabilities it is bringing!
@mastercactapus Thanks for your patience on this PR. I will take a look and get this merged in soon!
Hey @mastercactapus, I'm running into a timeout panic when a match is not detected:
I think we shouldn't panic here but rather do one of two things.
- Assume the VHS tape didn't work as intended. Stop immediately.
- Assume a timeout is a viable path, continue performing the actions in the tape.
I'm leaning towards option one, current the panic does do that but we should output a nicer message, similar to what it's currently showing without the stack trace:
Timeout waiting for "Wait+Line hi" to match hi
For option 2, timeout should not be considered a viable path IMO; it would be dangerous to keep executing commands in an unknown state (e.g., ssh connection failed, it may then execute commands locally). Timeout "should not" happen, so it's an unexpected state from the author's perspective, and I think the expected behavior would be to consider it failed and stop.
To do that/option 1: Is there a way to stop the VHS outside of a panic? I'd have opted to return an error, but command signatures don't have that -- and it seemed like a big change to refactor everything in a feature PR. It's probably worth the effort, but I think it should be done in a separate PR to keep things reviewable/manageable.
If there's already a built-in way to stop an error, I'm happy to make the update.
^ put another way, the panic was intentional to be a "stop with error" for functions that don't return an error
That makes absolute sense! We should error out if a timeout occurs. Yes, we'll likely need a way to bubble up and stop execution of the VHS program in this case. Perhaps we can add a method to mark the VHS instance as failed which would exit or cancel the context that we use when the user press ctrl+c
@denisonbarbosa we should definitely use this feature in our tapes once it will land!
I've been testing this branch for some CI/CD pipelines and it's worked very well for most of the jobs I've tested it on!
However, I noticed that Wait+Screen occasionally times out even when the job otherwise completed successfully and even after the Wait timeout has been extended significantly.
As a simulation of a command in a CI pipeline that generates a lot of line-based output, consider the following tape which enumerates words in the local words dictionary.
Output dict-words.webm
Output dict-words.txt
Require cat
Sleep 500ms
Type "# Enumerating words in the local dictionary" Enter
Type "nl < /usr/share/dict/words" Sleep 250ms Enter
Wait
Sleep 2s
This tape completes successfully, and the video correctly shows the last page of the dictionary followed by a new prompt line.
However, if I try to match the last line of output to determine success or failure, Wait+Screen will time out.
Here, the last word in my copy of /usr/share/dict/words is ZZZ, so I want to match its presence to determine success:
Output dict-words-ZZZ.webm
Output dict-words-ZZZ.txt
Require cat
Sleep 500ms
Type "# Enumerating words in the local dictionary" Enter
Type "nl < /usr/share/dict/words" Sleep 250ms Enter
Wait+Screen /ZZZ/
Sleep 2s
This fails with the following panic:
panic: timeout waiting for "Screen ZZZ" to match ZZZ; last value was: 478810 Zingiberaceae
478811 zingiberaceous
478812 zingiberene
478813 zingiberol
478814 zingiberone
478815 zingier
478816 zingiest
478817 zinging
478818 zings
478819 zingy
478820 zinjanthropi
478821 Zinjanthropus
478822 zinjanthropus
478823 Zink
478824 zink
478825 zinke
478826 zinked
478827 zinkenite
goroutine 1 [running]:
main.ExecuteWait({{0x10236e9, 0x4}, {0x0, 0x0}, {0xc00069cdc0, 0xa}}, 0xc0001302a0)
/home/justsh/github.com/mastercactapus/vhs/command.go:210 +0x4b8
main.Command.Execute({{0x10236e9, 0x4}, {0x0, 0x0}, {0xc00069cdc0, 0xa}}, 0xc0001302a0)
/home/justsh/github.com/mastercactapus/vhs/command.go:114 +0xae
main.Evaluate({0x14fc500, 0xc00069eac0}, {0xc00018a000, 0xd1}, {0x14f64e0, 0xc000078040}, {0xc000259b78, 0x1, 0x0?})
/home/justsh/github.com/mastercactapus/vhs/evaluator.go:139 +0xd53
main.glob..func2(0x1bf00a0, {0xc000699c40, 0x1, 0x1023799?})
/home/justsh/github.com/mastercactapus/vhs/main.go:97 +0x44e
github.com/spf13/cobra.(*Command).execute(0x1bf00a0, {0xc000036050, 0x1, 0x1})
/home/systemexit/.go/1.21.6/pkg/mod/github.com/spf13/[email protected]/command.go:983 +0xabc
github.com/spf13/cobra.(*Command).ExecuteC(0x1bf00a0)
/home/systemexit/.go/1.21.6/pkg/mod/github.com/spf13/[email protected]/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
/home/systemexit/.go/1.21.6/pkg/mod/github.com/spf13/[email protected]/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(...)
/home/systemexit/.go/1.21.6/pkg/mod/github.com/spf13/[email protected]/command.go:1032
main.main()
/home/justsh/github.com/mastercactapus/vhs/main.go:248 +0xc9
I noticed that the last screen of output mentioned in the panic stops short of the last page of the dictionary by about 1000 words, which feels similar to a buffer not being flushed somewhere.
$ nl < /usr/share/dict/words | grep -E 'zinkenite|ZZZ' | tee /dev/tty | cut -f1 | sort -nr | paste -sd- | xargs -t -L1 | bc
478827 zinkenite
479826 ZZZ
echo 479826-478827
999
Noting that the last value text mentioned in the panic seems to be consistent with the screens shown in the Output dict-words.txt from the my first test tape, I wondered if the calculations for vhs.Buffer() might be out of sync.
Specifically, vhs.Buffer() is different from vhs.CurrentLine() in that when vhs.Buffer() calls IBuffer.getLine() it uses Terminal.rows (the size of the viewport) but does not start at term.buffer.active.viewportY like vhs.CurrentLine() does.
Should vhs.Buffer() be calculated from the viewport instead? It makes sense that for small output those calculations would be equivalent, but with a full IBuffer you could get output from "the past" since the IBuffer.viewportY would have "scrolled down".
diff --git a/testing.go b/testing.go
index 8524684..e468e6b 100644
--- a/testing.go
+++ b/testing.go
@@ -55,7 +55,7 @@ func (v *VHS) SaveOutput() {
// Buffer returns the current buffer.
func (v *VHS) Buffer() ([]string, error) {
// Get the current buffer.
- buf, err := v.Page.Eval("() => Array(term.rows).fill(0).map((e, i) => term.buffer.active.getLine(i).translateToString().trimEnd())")
+ buf, err := v.Page.Eval("() => Array(term.rows).fill(0).map((e, i) => term.buffer.active.getLine(term.buffer.active.viewportY+i).translateToString().trimEnd())")
if err != nil {
return nil, fmt.Errorf("read buffer: %w", err)
}
After making the above change locally and re-running the tape with Wait+Screen /ZZZ/ it completes recording successfully and (as expected) the last screen in Output dict-words-ZZZ.txt also shows the final viewport and prompt for me.
I've just started using VHS and this feature is the one thing I need before adopting it. I have some commands that take varying amounts of time to run depending on the network. I'd really like to say: Wait until you see text "XYZ" instead of Sleep. Thank you!
As this PR hasn't been closed yet and it is now conflicting with main, I spent a bit of time today to fork the repo and merge the changes from mastercactapus and justsh .
I also set a GH pipeline to build the relevant binaries.
So, until this PR is part of this repo, If anyone is interested in the Wait feature, you can get the complied binary from the releases in my fork here or can also use eget with eget b-per/vhs to download the relevant binary for your machine.
@b-per I've got an error on your fork:
panic: timeout waiting for "Line" to match >$; last value was:
goroutine 1 [running]:
main.ExecuteWait({{0x1031214, 0x4}, {0x0, 0x0}, {0x10312b8, 0x4}}, 0xc0002b0000)
/home/runner/work/vhs/vhs/command.go:159 +0x4b8
main.Execute({{0x1031214, 0x4}, {0x0, 0x0}, {0x10312b8, 0x4}}, 0xc0002b0000)
/home/runner/work/vhs/vhs/command.go:27 +0xae
main.Evaluate({0x150af18, 0xc000463cc0}, {0xc0001ce000, 0x1e0}, {0x15050c0, 0xc000090028}, {0xc0006d1b78, 0x1, 0x0?})
/home/runner/work/vhs/vhs/evaluator.go:143 +0xd53
main.glob..func2(0x1bff500, {0xc00047fb20, 0x1, 0x1031244?})
/home/runner/work/vhs/vhs/main.go:99 +0x44e
github.com/spf13/cobra.(*Command).execute(0x1bff500, {0xc0000ac010, 0x1, 0x1})
/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:983 +0xabc
github.com/spf13/cobra.(*Command).ExecuteC(0x1bff500)
/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1039
github.com/spf13/cobra.(*Command).ExecuteContext(...)
/home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:1032
main.main()
/home/runner/work/vhs/vhs/main.go:250 +0xc9
My tape file:
# Where should we write the GIF?
Output demo.gif
Set Shell fish
# Set up a 1200x600 terminal with 46px font.
Set FontSize 25
Set Width 1500
Set Height 1000
Set BorderRadius 10
Type "secator x httpx testphp.vulnweb.com"
Sleep 500ms
Enter
Wait
Edit: nevermind, had to increase the WaitTimeout:
Set WaitPattern />$/
Set WaitTimeout 2m
Is there a way to just wait indefinitely ?
Yes, the panic behavior was discussed in this comment as well. (this might be why it wasn't merged at that time)
Other than that which happens only when the timeout is reached so kinda makes sense. Other than that, it's worked really well for me once I set a proper (longer) timeout.
Are there any updates on the status of this work? What are the next steps that are needed?
@mastercactapus @maaslalani
@spkane I updated and resolved the conflicts, the main remaining issue is that there's no good way to have a command return an error (i.e., other than panic), which is needed because WAIT introduces the need for something to fail/timeout possibly.
I'll look at opening a separate PR to allow command funcs to return an error, as I have a bit of time this AM
@spkane I updated and resolved the conflicts, the main remaining issue is that there's no good way to have a command return an error (i.e., other than panic), which is needed because WAIT introduces the need for something to fail/timeout possibly.
I'll look at opening a separate PR to allow command funcs to return an error, as I have a bit of time this AM
Thanks, @mastercactapus! I definitely want to test this logic out, as I often create demos where I am waiting for events to occur. I frequently don't know how long the command will take to complete, so currently, I have to do a bunch of testing and guesstimate how long to sleep and then hope that it works out for the given run.