Normalize logging, output, and error reporting

- Return errors as errors, not print to stdout and return nil
- Ensure errors and warnings are always written to stderr, not stout
- Do not print progress to stdout unless stdout is a terminal
This commit is contained in:
Mislav Marohnić 2021-08-12 14:37:23 +02:00
parent 41e223869e
commit 20d75f0ff9
9 changed files with 52 additions and 51 deletions

View file

@ -43,8 +43,7 @@ func Code(codespaceName string) error {
codespace, err := codespaces.ChooseCodespace(ctx, apiClient, user)
if err != nil {
if err == codespaces.ErrNoCodespaces {
fmt.Println(err.Error())
return nil
return err
}
return fmt.Errorf("error choosing codespace: %v", err)
}

View file

@ -2,6 +2,7 @@ package main
import (
"context"
"errors"
"fmt"
"os"
"strings"
@ -9,6 +10,7 @@ import (
"github.com/AlecAivazis/survey/v2"
"github.com/fatih/camelcase"
"github.com/github/ghcs/api"
"github.com/github/ghcs/cmd/ghcs/output"
"github.com/spf13/cobra"
)
@ -17,7 +19,7 @@ var repo, branch, machine string
func newCreateCmd() *cobra.Command {
createCmd := &cobra.Command{
Use: "create",
Short: "Create a GitHub Codespace.",
Short: "Create a GitHub Codespace.",
RunE: func(cmd *cobra.Command, args []string) error {
return Create()
},
@ -39,6 +41,7 @@ func Create() error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
locationCh := getLocation(ctx, apiClient)
userCh := getUser(ctx, apiClient)
log := output.NewLogger(os.Stdout, os.Stderr, false)
repo, err := getRepoName()
if err != nil {
@ -69,18 +72,17 @@ func Create() error {
return fmt.Errorf("error getting machine type: %v", err)
}
if machine == "" {
fmt.Println("There are no available machine types for this repository")
return nil
return errors.New("There are no available machine types for this repository")
}
fmt.Println("Creating your codespace...")
log.Println("Creating your codespace...")
codespace, err := apiClient.CreateCodespace(ctx, userResult.User, repository, machine, branch, locationResult.Location)
if err != nil {
return fmt.Errorf("error creating codespace: %v", err)
}
fmt.Println("Codespace created: " + codespace.Name)
log.Printf("Codespace created: %s\n", codespace.Name)
return nil
}

View file

@ -7,6 +7,7 @@ import (
"os"
"github.com/github/ghcs/api"
"github.com/github/ghcs/cmd/ghcs/output"
"github.com/github/ghcs/internal/codespaces"
"github.com/spf13/cobra"
)
@ -58,6 +59,7 @@ func init() {
func Delete(codespaceName string) error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
ctx := context.Background()
log := output.NewLogger(os.Stdout, os.Stderr, false)
user, err := apiClient.GetUser(ctx)
if err != nil {
@ -73,7 +75,7 @@ func Delete(codespaceName string) error {
return fmt.Errorf("error deleting codespace: %v", err)
}
fmt.Println("Codespace deleted.")
log.Println("Codespace deleted.")
return List(&ListOptions{})
}
@ -81,6 +83,7 @@ func Delete(codespaceName string) error {
func DeleteAll() error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
ctx := context.Background()
log := output.NewLogger(os.Stdout, os.Stderr, false)
user, err := apiClient.GetUser(ctx)
if err != nil {
@ -102,7 +105,7 @@ func DeleteAll() error {
return fmt.Errorf("error deleting codespace: %v", err)
}
fmt.Printf("Codespace deleted: %s\n", c.Name)
log.Printf("Codespace deleted: %s\n", c.Name)
}
return List(&ListOptions{})
@ -111,6 +114,7 @@ func DeleteAll() error {
func DeleteByRepo(repo string) error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
ctx := context.Background()
log := output.NewLogger(os.Stdout, os.Stderr, false)
user, err := apiClient.GetUser(ctx)
if err != nil {
@ -138,11 +142,11 @@ func DeleteByRepo(repo string) error {
return fmt.Errorf("error deleting codespace: %v", err)
}
fmt.Printf("Codespace deleted: %s\n", c.Name)
log.Printf("Codespace deleted: %s\n", c.Name)
}
if !deleted {
fmt.Printf("No codespace was found for repository: %s\n", repo)
return fmt.Errorf("No codespace was found for repository: %s", repo)
}
return List(&ListOptions{})

View file

@ -48,11 +48,6 @@ func List(opts *ListOptions) error {
return fmt.Errorf("error getting codespaces: %v", err)
}
if len(codespaces) == 0 {
fmt.Println("You have no codespaces.")
return nil
}
table := output.NewTable(os.Stdout, opts.AsJSON)
table.SetHeader([]string{"Name", "Repository", "Branch", "State", "Created At"})
for _, codespace := range codespaces {

View file

@ -7,6 +7,7 @@ import (
"os"
"github.com/github/ghcs/api"
"github.com/github/ghcs/cmd/ghcs/output"
"github.com/github/ghcs/internal/codespaces"
"github.com/spf13/cobra"
)
@ -38,6 +39,7 @@ func init() {
func Logs(tail bool, codespaceName string) error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
ctx := context.Background()
log := output.NewLogger(os.Stdout, os.Stderr, false)
user, err := apiClient.GetUser(ctx)
if err != nil {
@ -49,7 +51,7 @@ func Logs(tail bool, codespaceName string) error {
return fmt.Errorf("get or choose codespace: %v", err)
}
lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace)
lsclient, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, token, codespace)
if err != nil {
return fmt.Errorf("connecting to liveshare: %v", err)
}

View file

@ -22,7 +22,7 @@ var rootCmd = &cobra.Command{
func Execute() {
if os.Getenv("GITHUB_TOKEN") == "" {
fmt.Println("The GITHUB_TOKEN environment variable is required. Create a Personal Access Token at https://github.com/settings/tokens/new?scopes=repo and make sure to enable SSO for the GitHub organization after creating the token.")
fmt.Fprintln(os.Stderr, "The GITHUB_TOKEN environment variable is required. Create a Personal Access Token at https://github.com/settings/tokens/new?scopes=repo and make sure to enable SSO for the GitHub organization after creating the token.")
os.Exit(1)
}

View file

@ -61,15 +61,14 @@ func Ports(opts *PortsOptions) error {
codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, opts.CodespaceName)
if err != nil {
if err == codespaces.ErrNoCodespaces {
fmt.Println(err.Error())
return nil
return err
}
return fmt.Errorf("error choosing codespace: %v", err)
}
devContainerCh := getDevContainer(ctx, apiClient, codespace)
liveShareClient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace)
liveShareClient, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, token, codespace)
if err != nil {
return fmt.Errorf("error connecting to liveshare: %v", err)
}
@ -177,7 +176,8 @@ func NewPortsPublicCmd() *cobra.Command {
return errors.New("[codespace_name] [source] port number are required.")
}
return updatePortVisibility(args[0], args[1], true)
log := output.NewLogger(os.Stdout, os.Stderr, false)
return updatePortVisibility(log, args[0], args[1], true)
},
}
}
@ -192,12 +192,13 @@ func NewPortsPrivateCmd() *cobra.Command {
return errors.New("[codespace_name] [source] port number are required.")
}
return updatePortVisibility(args[0], args[1], false)
log := output.NewLogger(os.Stdout, os.Stderr, false)
return updatePortVisibility(log, args[0], args[1], false)
},
}
}
func updatePortVisibility(codespaceName, sourcePort string, public bool) error {
func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, public bool) error {
ctx := context.Background()
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
@ -216,7 +217,7 @@ func updatePortVisibility(codespaceName, sourcePort string, public bool) error {
return fmt.Errorf("error getting codespace: %v", err)
}
lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace)
lsclient, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, token, codespace)
if err != nil {
return fmt.Errorf("error connecting to liveshare: %v", err)
}
@ -236,11 +237,10 @@ func updatePortVisibility(codespaceName, sourcePort string, public bool) error {
}
state := "PUBLIC"
if public == false {
if !public {
state = "PRIVATE"
}
fmt.Println(fmt.Sprintf("Port %s is now %s.", sourcePort, state))
log.Printf("Port %s is now %s.\n", sourcePort, state)
return nil
}
@ -254,12 +254,14 @@ func NewPortsForwardCmd() *cobra.Command {
if len(args) < 3 {
return errors.New("[codespace_name] [source] [dst] port number are required.")
}
return forwardPort(args[0], args[1], args[2])
log := output.NewLogger(os.Stdout, os.Stderr, false)
return forwardPort(log, args[0], args[1], args[2])
},
}
}
func forwardPort(codespaceName, sourcePort, destPort string) error {
func forwardPort(log *output.Logger, codespaceName, sourcePort, destPort string) error {
ctx := context.Background()
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
@ -278,7 +280,7 @@ func forwardPort(codespaceName, sourcePort, destPort string) error {
return fmt.Errorf("error getting codespace: %v", err)
}
lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace)
lsclient, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, token, codespace)
if err != nil {
return fmt.Errorf("error connecting to liveshare: %v", err)
}
@ -302,7 +304,7 @@ func forwardPort(codespaceName, sourcePort, destPort string) error {
return fmt.Errorf("error sharing source port: %v", err)
}
fmt.Println("Forwarding port: " + sourcePort + " -> " + destPort)
log.Println("Forwarding port: " + sourcePort + " -> " + destPort)
portForwarder := liveshare.NewPortForwarder(lsclient, server, dstPortInt)
if err := portForwarder.Start(ctx); err != nil {
return fmt.Errorf("error forwarding port: %v", err)

View file

@ -9,6 +9,7 @@ import (
"time"
"github.com/github/ghcs/api"
"github.com/github/ghcs/cmd/ghcs/output"
"github.com/github/ghcs/internal/codespaces"
"github.com/github/go-liveshare"
"github.com/spf13/cobra"
@ -40,6 +41,7 @@ func init() {
func SSH(sshProfile, codespaceName string, sshServerPort int) error {
apiClient := api.New(os.Getenv("GITHUB_TOKEN"))
ctx := context.Background()
log := output.NewLogger(os.Stdout, os.Stderr, false)
user, err := apiClient.GetUser(ctx)
if err != nil {
@ -51,7 +53,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error {
return fmt.Errorf("get or choose codespace: %v", err)
}
lsclient, err := codespaces.ConnectToLiveshare(ctx, apiClient, token, codespace)
lsclient, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, token, codespace)
if err != nil {
return fmt.Errorf("error connecting to liveshare: %v", err)
}
@ -61,7 +63,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error {
return fmt.Errorf("error creating liveshare terminal: %v", err)
}
fmt.Println("Preparing SSH...")
log.Println("Preparing SSH...")
if sshProfile == "" {
containerID, err := getContainerID(ctx, terminal)
if err != nil {
@ -71,8 +73,6 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error {
if err := setupSSH(ctx, terminal, containerID, codespace.RepositoryName); err != nil {
return fmt.Errorf("error creating ssh server: %v", err)
}
fmt.Printf("\n")
}
tunnelPort, tunnelClosed, err := codespaces.MakeSSHTunnel(ctx, lsclient, sshServerPort)
@ -88,7 +88,7 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error {
usingCustomPort := tunnelPort == sshServerPort
connClosed := codespaces.ConnectToTunnel(ctx, tunnelPort, connectDestination, usingCustomPort)
fmt.Println("Ready...")
log.Println("Ready...")
select {
case err := <-tunnelClosed:
if err != nil {
@ -104,7 +104,6 @@ func SSH(sshProfile, codespaceName string, sshServerPort int) error {
}
func getContainerID(ctx context.Context, terminal *liveshare.Terminal) (string, error) {
fmt.Print(".")
cmd := terminal.NewCommand(
"/",
"/usr/bin/docker ps -aq --filter label=Type=codespaces --filter status=running",
@ -114,17 +113,14 @@ func getContainerID(ctx context.Context, terminal *liveshare.Terminal) (string,
return "", fmt.Errorf("error running command: %v", err)
}
fmt.Print(".")
scanner := bufio.NewScanner(stream)
scanner.Scan()
fmt.Print(".")
containerID := scanner.Text()
if err := scanner.Err(); err != nil {
return "", fmt.Errorf("error scanning stream: %v", err)
}
fmt.Print(".")
if err := stream.Close(); err != nil {
return "", fmt.Errorf("error closing stream: %v", err)
}
@ -135,7 +131,6 @@ func getContainerID(ctx context.Context, terminal *liveshare.Terminal) (string,
func setupSSH(ctx context.Context, terminal *liveshare.Terminal, containerID, repositoryName string) error {
setupBashProfileCmd := fmt.Sprintf(`echo "cd /workspaces/%v; export $(cat /workspaces/.codespaces/shared/.env | xargs); exec /bin/zsh;" > /home/codespace/.bash_profile`, repositoryName)
fmt.Print(".")
compositeCommand := []string{setupBashProfileCmd}
cmd := terminal.NewCommand(
"/",
@ -146,7 +141,6 @@ func setupSSH(ctx context.Context, terminal *liveshare.Terminal, containerID, re
return fmt.Errorf("error running command: %v", err)
}
fmt.Print(".")
if err := stream.Close(); err != nil {
return fmt.Errorf("error closing stream: %v", err)
}

View file

@ -57,9 +57,14 @@ func ChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*
return codespace, nil
}
func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, codespace *api.Codespace) (client *liveshare.Client, err error) {
type logger interface {
Print(v ...interface{}) (int, error)
Println(v ...interface{}) (int, error)
}
func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, token string, codespace *api.Codespace) (client *liveshare.Client, err error) {
if codespace.Environment.State != api.CodespaceEnvironmentStateAvailable {
fmt.Println("Starting your codespace...") // TODO(josebalius): better way of notifying of events
log.Println("Starting your codespace...")
if err := apiClient.StartCodespace(ctx, token, codespace); err != nil {
return nil, fmt.Errorf("error starting codespace: %v", err)
}
@ -69,7 +74,7 @@ func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, c
for codespace.Environment.Connection.SessionID == "" || codespace.Environment.State != api.CodespaceEnvironmentStateAvailable {
if retries > 1 {
if retries%2 == 0 {
fmt.Print(".")
log.Print(".")
}
time.Sleep(1 * time.Second)
@ -88,10 +93,10 @@ func ConnectToLiveshare(ctx context.Context, apiClient *api.API, token string, c
}
if retries >= 2 {
fmt.Print("\n")
log.Print("\n")
}
fmt.Println("Connecting to your codespace...")
log.Println("Connecting to your codespace...")
lsclient, err := liveshare.NewClient(
liveshare.WithConnection(liveshare.Connection{
@ -117,10 +122,8 @@ func GetOrChooseCodespace(ctx context.Context, apiClient *api.API, user *api.Use
codespace, err = ChooseCodespace(ctx, apiClient, user)
if err != nil {
if err == ErrNoCodespaces {
fmt.Println(err.Error())
return nil, "", nil
return nil, "", err
}
return nil, "", fmt.Errorf("choosing codespace: %v", err)
}
codespaceName = codespace.Name