Spinner: Context ignored/dropped under Accessible mode

[polds]

Describe the bug
The Context() method only passes the context to the underlying bubbletea program. If you set the Spinner to use Accessible mode then the context isn't used.

Also worth noting, even though Run() returns an error, it is not possible to have a non-nil error:

huh/spinner/spinner.go

Lines 138 to 169 in acfe24c

	// Run runs the spinner.
	func (s *Spinner) Run() error {
	if s.accessible {
	return s.runAccessible()
	}
	p := tea.NewProgram(s, tea.WithContext(s.ctx), tea.WithOutput(os.Stderr))
	if s.ctx == nil {
	go func() {
	s.action()
	p.Quit()
	}()
	}
	_, _ = p.Run()
	return nil
	}
	// runAccessible runs the spinner in an accessible mode (statically).
	func (s *Spinner) runAccessible() error {
	s.output.HideCursor()
	frame := s.spinner.Style.Render("...")
	title := s.titleStyle.Render(s.title)
	fmt.Println(title + frame)
	s.action()
	s.output.ShowCursor()
	s.output.CursorBack(len(frame) + len(title))
	return nil
	}

So I do see the expected behavior of fast cancellation on context cancellation, just with no error. But when in Accessible mode the context is ignored.

To Reproduce
Steps to reproduce the behavior:

https://go.dev/play/p/MTobz4rLf7t

package main

import (
	"context"
	"testing"
	"time"

	"github.com/charmbracelet/huh/spinner"
)


func TestContextCancellation(t *testing.T) {
        ctx := context.Background()
	t.Run("works", func(t *testing.T) {
		ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
		t.Cleanup(cancel)

		var i int
		// Note: Spinner does not correctly bubble up the deadline exceeded error, but it
		//  does correctly stop execution.
		_ = spinner.New().Action(func() {
			time.Sleep(100 * time.Millisecond)
			i++
		}).Context(ctx).Run()
		if i != 0 {
			t.Errorf("Got count of %d, but expected 0", i)
		}
	})
	t.Run("bug", func(t *testing.T) {
		ctx, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
		t.Cleanup(cancel)

		var i int
		// Note: Spinner does not correctly bubble up the deadline exceeded error, but it
		//  does correctly stop execution.
		_ = spinner.New().Action(func() {
			time.Sleep(100 * time.Millisecond)
			i++
		}).Context(ctx).Accessible(true).Run()
		if i != 0 {
			t.Errorf("Got count of %d, but expected 0", i)
		}
	})
}

Expected behavior

• Run() returns a context.Cancelled error type
• Accessible(true) still handles context cancellation

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

[maaslalani]

Hey @polds, thanks so much for the issue!

This is now fixed (let me know if there's any problems though):

https://github.com/charmbracelet/huh/blob/main/spinner/spinner.go#L160-L192

[testinfected]

@maaslalani with your change, note that there is an inconsistency between run which won't run the action if a context is set, and runAccessible, which always run the action.

I believe run should always run the action as well, even if a context is set. My take is that it would make the spinner easier to use. It would also help return proper errors.

For instance, we could set a flag when CTRL+C is pressed, and return an error on p.Quit() in this case.

Something like this:

func (s *Spinner) run() error {
	if s.accessible {
		return s.runAccessible()
	}

	p := tea.NewProgram(s, tea.WithContext(s.ctx), tea.WithOutput(os.Stderr))

        var failure error
	go func() {
		failure = s.action()
		p.Quit()
	}()

	_, err = p.Run()

       switch {
       case s.interrupted: 
               return errors.New("operation interrupted")
       case failure != nil:
               return failure
       default:
               return err
       }
}

[maaslalani]

Yeah you're correct @testinfected, I think if the action is provided we should run it and if it's not we should simply wait until the context is cancelled on both TUI and accessible mode.