[bug] Errors interrupt spinner, producing screen output harder to interpret

[heaths]

When an error occurs, any spinner text gets broken up and somewhat duplicated across lines because the error is written to stderr (or stdout, whichever) just like the spinner. The main culprit seems to because the spinner isn't stopped first, which is what we do in the GitHub CLI.

This:

// Deploys the Azure infrastructure for the specified project
func (m *Manager) Deploy(ctx context.Context, plan *DeploymentPlan) (*DeployResult, error) {
	// Apply the infrastructure deployment
	deployResult, err := m.provider.Deploy(ctx, plan)
	if err != nil {
		return nil, fmt.Errorf("error deploying infrastructure: %w", err)
	}

	if err := UpdateEnvironment(m.env, deployResult.Deployment.Outputs); err != nil {
		return nil, fmt.Errorf("updating environment with deployment outputs: %w", err)
	}

	// make sure any spinner is stopped
	m.console.StopSpinner(ctx, "", input.StepDone)

	return deployResult, nil
}

Should be more like this:

// Deploys the Azure infrastructure for the specified project
func (m *Manager) Deploy(ctx context.Context, plan *DeploymentPlan) (*DeployResult, error) {
        // make sure any spinner is stopped
        defer  m.console.StopSpinner(ctx, "", input.StepDone)

	// Apply the infrastructure deployment
	deployResult, err := m.provider.Deploy(ctx, plan)
	if err != nil {
		return nil, fmt.Errorf("error deploying infrastructure: %w", err)
	}

	if err := UpdateEnvironment(m.env, deployResult.Deployment.Outputs); err != nil {
		return nil, fmt.Errorf("updating environment with deployment outputs: %w", err)
	}

	return deployResult, nil
}

Since you're only return an error, you could use defer to stop the spinner. The trickier part is when you have any code actally writing the error to stderr/stdout. Then you want to make sure you're more explicit about stopping the error.