From fb5a35568ca44340ba6e332452bca6873baae62d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 15 Sep 2021 13:58:10 +0200 Subject: [PATCH 1/8] Ensure original errors are wrapped with "%w" instead of "%v" --- api/api.go | 92 +++++++++++++++---------------- cmd/ghcs/code.go | 4 +- cmd/ghcs/common.go | 12 ++-- cmd/ghcs/create.go | 20 +++---- cmd/ghcs/delete.go | 22 ++++---- cmd/ghcs/list.go | 4 +- cmd/ghcs/logs.go | 12 ++-- cmd/ghcs/ports.go | 40 +++++++------- cmd/ghcs/ssh.go | 12 ++-- internal/codespaces/codespaces.go | 6 +- internal/codespaces/ssh.go | 4 +- internal/codespaces/states.go | 14 ++--- 12 files changed, 121 insertions(+), 121 deletions(-) diff --git a/api/api.go b/api/api.go index ad69f23fc..47948bbba 100644 --- a/api/api.go +++ b/api/api.go @@ -40,19 +40,19 @@ type User struct { func (a *API) GetUser(ctx context.Context) (*User, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/user", nil) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) resp, err := a.do(ctx, req, "/user") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -61,7 +61,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { var response User if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return &response, nil @@ -72,7 +72,7 @@ func jsonErrorResponse(b []byte) error { Message string `json:"message"` } if err := json.Unmarshal(b, &response); err != nil { - return fmt.Errorf("error unmarshaling error response: %v", err) + return fmt.Errorf("error unmarshaling error response: %w", err) } return errors.New(response.Message) @@ -85,19 +85,19 @@ type Repository struct { func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/repos/"+strings.ToLower(nwo), nil) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) resp, err := a.do(ctx, req, "/repos/*") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -106,7 +106,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error var response Repository if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return &response, nil @@ -154,19 +154,19 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, err http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", nil, ) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -177,7 +177,7 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, err Codespaces []*Codespace `json:"codespaces"` } if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return response.Codespaces, nil } @@ -193,7 +193,7 @@ type getCodespaceTokenResponse struct { func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName string) (string, error) { reqBody, err := json.Marshal(getCodespaceTokenRequest{true}) if err != nil { - return "", fmt.Errorf("error preparing request body: %v", err) + return "", fmt.Errorf("error preparing request body: %w", err) } req, err := http.NewRequest( @@ -202,19 +202,19 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s bytes.NewBuffer(reqBody), ) if err != nil { - return "", fmt.Errorf("error creating request: %v", err) + return "", fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*/token") if err != nil { - return "", fmt.Errorf("error making request: %v", err) + return "", fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("error reading response body: %v", err) + return "", fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -223,7 +223,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s var response getCodespaceTokenResponse if err := json.Unmarshal(b, &response); err != nil { - return "", fmt.Errorf("error unmarshaling response: %v", err) + return "", fmt.Errorf("error unmarshaling response: %w", err) } return response.RepositoryToken, nil @@ -236,19 +236,19 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) nil, ) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } req.Header.Set("Authorization", "Bearer "+token) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -257,7 +257,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) var response Codespace if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return &response, nil @@ -270,19 +270,19 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes nil, ) if err != nil { - return fmt.Errorf("error creating request: %v", err) + return fmt.Errorf("error creating request: %w", err) } req.Header.Set("Authorization", "Bearer "+token) resp, err := a.do(ctx, req, "/vscs_internal/proxy/environments/*/start") if err != nil { - return fmt.Errorf("error making request: %v", err) + return fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("error reading response body: %v", err) + return fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -308,18 +308,18 @@ type getCodespaceRegionLocationResponse struct { func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { req, err := http.NewRequest(http.MethodGet, "https://online.visualstudio.com/api/v1/locations", nil) if err != nil { - return "", fmt.Errorf("error creating request: %v", err) + return "", fmt.Errorf("error creating request: %w", err) } resp, err := a.do(ctx, req, req.URL.String()) if err != nil { - return "", fmt.Errorf("error making request: %v", err) + return "", fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("error reading response body: %v", err) + return "", fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -328,7 +328,7 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { var response getCodespaceRegionLocationResponse if err := json.Unmarshal(b, &response); err != nil { - return "", fmt.Errorf("error unmarshaling response: %v", err) + return "", fmt.Errorf("error unmarshaling response: %w", err) } return response.Current, nil @@ -342,7 +342,7 @@ type SKU struct { func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Repository, branch, location string) ([]*SKU, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/vscs_internal/user/"+user.Login+"/skus", nil) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } q := req.URL.Query() @@ -354,13 +354,13 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep a.setHeaders(req) resp, err := a.do(ctx, req, "/vscs_internal/user/*/skus") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -371,7 +371,7 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep SKUs []*SKU `json:"skus"` } if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return response.SKUs, nil @@ -387,24 +387,24 @@ type createCodespaceRequest struct { func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repository, sku, branch, location string) (*Codespace, error) { requestBody, err := json.Marshal(createCodespaceRequest{repository.ID, branch, location, sku}) if err != nil { - return nil, fmt.Errorf("error marshaling request: %v", err) + return nil, fmt.Errorf("error marshaling request: %w", err) } req, err := http.NewRequest(http.MethodPost, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces", bytes.NewBuffer(requestBody)) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode > http.StatusAccepted { @@ -413,7 +413,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos var response Codespace if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } return &response, nil @@ -422,20 +422,20 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceName string) error { req, err := http.NewRequest(http.MethodDelete, githubAPI+"/vscs_internal/user/"+user.Login+"/codespaces/"+codespaceName, nil) if err != nil { - return fmt.Errorf("error creating request: %v", err) + return fmt.Errorf("error creating request: %w", err) } req.Header.Set("Authorization", "Bearer "+token) resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { - return fmt.Errorf("error making request: %v", err) + return fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() if resp.StatusCode > http.StatusAccepted { b, err := ioutil.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("error reading response body: %v", err) + return fmt.Errorf("error reading response body: %w", err) } return jsonErrorResponse(b) } @@ -450,7 +450,7 @@ type getCodespaceRepositoryContentsResponse struct { func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Codespace, path string) ([]byte, error) { req, err := http.NewRequest(http.MethodGet, githubAPI+"/repos/"+codespace.RepositoryNWO+"/contents/"+path, nil) if err != nil { - return nil, fmt.Errorf("error creating request: %v", err) + return nil, fmt.Errorf("error creating request: %w", err) } q := req.URL.Query() @@ -460,7 +460,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod a.setHeaders(req) resp, err := a.do(ctx, req, "/repos/*/contents/*") if err != nil { - return nil, fmt.Errorf("error making request: %v", err) + return nil, fmt.Errorf("error making request: %w", err) } defer resp.Body.Close() @@ -470,7 +470,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod b, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("error reading response body: %v", err) + return nil, fmt.Errorf("error reading response body: %w", err) } if resp.StatusCode != http.StatusOK { @@ -479,12 +479,12 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod var response getCodespaceRepositoryContentsResponse if err := json.Unmarshal(b, &response); err != nil { - return nil, fmt.Errorf("error unmarshaling response: %v", err) + return nil, fmt.Errorf("error unmarshaling response: %w", err) } decoded, err := base64.StdEncoding.DecodeString(response.Content) if err != nil { - return nil, fmt.Errorf("error decoding content: %v", err) + return nil, fmt.Errorf("error decoding content: %w", err) } return decoded, nil diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 3bd67053d..bdad09828 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -42,7 +42,7 @@ func code(codespaceName string, useInsiders bool) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } if codespaceName == "" { @@ -51,7 +51,7 @@ func code(codespaceName string, useInsiders bool) error { if err == errNoCodespaces { return err } - return fmt.Errorf("error choosing codespace: %v", err) + return fmt.Errorf("error choosing codespace: %w", err) } codespaceName = codespace.Name } diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go index bfcb67496..2e716e897 100644 --- a/cmd/ghcs/common.go +++ b/cmd/ghcs/common.go @@ -20,7 +20,7 @@ var errNoCodespaces = errors.New("You have no codespaces.") func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*api.Codespace, error) { codespaces, err := apiClient.ListCodespaces(ctx, user) if err != nil { - return nil, fmt.Errorf("error getting codespaces: %v", err) + return nil, fmt.Errorf("error getting codespaces: %w", err) } if len(codespaces) == 0 { @@ -54,7 +54,7 @@ func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (* Codespace string } if err := ask(sshSurvey, &answers); err != nil { - return nil, fmt.Errorf("error getting answers: %v", err) + return nil, fmt.Errorf("error getting answers: %w", err) } codespace := codespacesByName[answers.Codespace] @@ -70,23 +70,23 @@ func getOrChooseCodespace(ctx context.Context, apiClient *api.API, user *api.Use if err == errNoCodespaces { return nil, "", err } - return nil, "", fmt.Errorf("choosing codespace: %v", err) + return nil, "", fmt.Errorf("choosing codespace: %w", err) } codespaceName = codespace.Name token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) if err != nil { - return nil, "", fmt.Errorf("getting codespace token: %v", err) + return nil, "", fmt.Errorf("getting codespace token: %w", err) } } else { token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) if err != nil { - return nil, "", fmt.Errorf("getting codespace token for given codespace: %v", err) + return nil, "", fmt.Errorf("getting codespace token for given codespace: %w", err) } codespace, err = apiClient.GetCodespace(ctx, token, user.Login, codespaceName) if err != nil { - return nil, "", fmt.Errorf("getting full codespace details: %v", err) + return nil, "", fmt.Errorf("getting full codespace details: %w", err) } } diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 093450e7d..9f9b1da7a 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -55,31 +55,31 @@ func create(opts *createOptions) error { repo, err := getRepoName(opts.repo) if err != nil { - return fmt.Errorf("error getting repository name: %v", err) + return fmt.Errorf("error getting repository name: %w", err) } branch, err := getBranchName(opts.branch) if err != nil { - return fmt.Errorf("error getting branch name: %v", err) + return fmt.Errorf("error getting branch name: %w", err) } repository, err := apiClient.GetRepository(ctx, repo) if err != nil { - return fmt.Errorf("error getting repository: %v", err) + return fmt.Errorf("error getting repository: %w", err) } locationResult := <-locationCh if locationResult.Err != nil { - return fmt.Errorf("error getting codespace region location: %v", locationResult.Err) + return fmt.Errorf("error getting codespace region location: %w", locationResult.Err) } userResult := <-userCh if userResult.Err != nil { - return fmt.Errorf("error getting codespace user: %v", userResult.Err) + return fmt.Errorf("error getting codespace user: %w", userResult.Err) } machine, err := getMachineName(ctx, opts.machine, userResult.User, repository, branch, locationResult.Location, apiClient) if err != nil { - return fmt.Errorf("error getting machine type: %v", err) + return fmt.Errorf("error getting machine type: %w", err) } if machine == "" { return errors.New("There are no available machine types for this repository") @@ -89,7 +89,7 @@ func create(opts *createOptions) error { codespace, err := apiClient.CreateCodespace(ctx, userResult.User, repository, machine, branch, locationResult.Location) if err != nil { - return fmt.Errorf("error creating codespace: %v", err) + return fmt.Errorf("error creating codespace: %w", err) } if opts.showStatus { @@ -154,7 +154,7 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use } if err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller); err != nil { - return fmt.Errorf("failed to poll state changes from codespace: %v", err) + return fmt.Errorf("failed to poll state changes from codespace: %w", err) } return nil @@ -228,7 +228,7 @@ func getBranchName(branch string) (string, error) { func getMachineName(ctx context.Context, machine string, user *api.User, repo *api.Repository, branch, location string, apiClient *api.API) (string, error) { skus, err := apiClient.GetCodespacesSKUs(ctx, user, repo, branch, location) if err != nil { - return "", fmt.Errorf("error requesting machine instance types: %v", err) + return "", fmt.Errorf("error requesting machine instance types: %w", err) } // if user supplied a machine type, it must be valid @@ -278,7 +278,7 @@ func getMachineName(ctx context.Context, machine string, user *api.User, repo *a var skuAnswers struct{ SKU string } if err := ask(skuSurvey, &skuAnswers); err != nil { - return "", fmt.Errorf("error getting SKU: %v", err) + return "", fmt.Errorf("error getting SKU: %w", err) } sku := skuByName[skuAnswers.SKU] diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 7800a13c0..100793010 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -55,16 +55,16 @@ func delete_(log *output.Logger, codespaceName string) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { - return fmt.Errorf("get or choose codespace: %v", err) + return fmt.Errorf("get or choose codespace: %w", err) } if err := apiClient.DeleteCodespace(ctx, user, token, codespace.Name); err != nil { - return fmt.Errorf("error deleting codespace: %v", err) + return fmt.Errorf("error deleting codespace: %w", err) } log.Println("Codespace deleted.") @@ -78,22 +78,22 @@ func deleteAll(log *output.Logger) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespaces, err := apiClient.ListCodespaces(ctx, user) if err != nil { - return fmt.Errorf("error getting codespaces: %v", err) + return fmt.Errorf("error getting codespaces: %w", err) } for _, c := range codespaces { token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) if err != nil { - return fmt.Errorf("error getting codespace token: %v", err) + return fmt.Errorf("error getting codespace token: %w", err) } if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { - return fmt.Errorf("error deleting codespace: %v", err) + return fmt.Errorf("error deleting codespace: %w", err) } log.Printf("Codespace deleted: %s\n", c.Name) @@ -108,12 +108,12 @@ func deleteByRepo(log *output.Logger, repo string) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespaces, err := apiClient.ListCodespaces(ctx, user) if err != nil { - return fmt.Errorf("error getting codespaces: %v", err) + return fmt.Errorf("error getting codespaces: %w", err) } var deleted bool @@ -125,11 +125,11 @@ func deleteByRepo(log *output.Logger, repo string) error { token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) if err != nil { - return fmt.Errorf("error getting codespace token: %v", err) + return fmt.Errorf("error getting codespace token: %w", err) } if err := apiClient.DeleteCodespace(ctx, user, token, c.Name); err != nil { - return fmt.Errorf("error deleting codespace: %v", err) + return fmt.Errorf("error deleting codespace: %w", err) } log.Printf("Codespace deleted: %s\n", c.Name) diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index ee26e3013..a315d12ed 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -41,12 +41,12 @@ func list(opts *listOptions) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespaces, err := apiClient.ListCodespaces(ctx, user) if err != nil { - return fmt.Errorf("error getting codespaces: %v", err) + return fmt.Errorf("error getting codespaces: %w", err) } table := output.NewTable(os.Stdout, opts.asJSON) diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index a685a364d..6b5e2f875 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -49,17 +49,17 @@ func logs(ctx context.Context, log *output.Logger, codespaceName string, follow user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("getting user: %v", err) + return fmt.Errorf("getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { - return fmt.Errorf("get or choose codespace: %v", err) + return fmt.Errorf("get or choose codespace: %w", err) } session, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("connecting to Live Share: %v", err) + return fmt.Errorf("connecting to Live Share: %w", err) } // Ensure local port is listening before client (getPostCreateOutput) connects. @@ -72,7 +72,7 @@ func logs(ctx context.Context, log *output.Logger, codespaceName string, follow remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) if err != nil { - return fmt.Errorf("error getting ssh server details: %v", err) + return fmt.Errorf("error getting ssh server details: %w", err) } cmdType := "cat" @@ -98,10 +98,10 @@ func logs(ctx context.Context, log *output.Logger, codespaceName string, follow select { case err := <-tunnelClosed: - return fmt.Errorf("connection closed: %v", err) + return fmt.Errorf("connection closed: %w", err) case err := <-cmdDone: if err != nil { - return fmt.Errorf("error retrieving logs: %v", err) + return fmt.Errorf("error retrieving logs: %w", err) } return nil // success diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index f6d1f6e2a..12c631c04 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -58,7 +58,7 @@ func ports(codespaceName string, asJSON bool) error { user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) @@ -67,20 +67,20 @@ func ports(codespaceName string, asJSON bool) error { if err == errNoCodespaces { return err } - return fmt.Errorf("error choosing codespace: %v", err) + return fmt.Errorf("error choosing codespace: %w", err) } devContainerCh := getDevContainer(ctx, apiClient, codespace) session, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("error connecting to Live Share: %v", err) + return fmt.Errorf("error connecting to Live Share: %w", err) } log.Println("Loading ports...") ports, err := session.GetSharedServers(ctx) if err != nil { - return fmt.Errorf("error getting ports of shared servers: %v", err) + return fmt.Errorf("error getting ports of shared servers: %w", err) } devContainerResult := <-devContainerCh @@ -130,7 +130,7 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod go func() { contents, err := apiClient.GetCodespaceRepositoryContents(ctx, codespace, ".devcontainer/devcontainer.json") if err != nil { - ch <- devContainerResult{nil, fmt.Errorf("error getting content: %v", err)} + ch <- devContainerResult{nil, fmt.Errorf("error getting content: %w", err)} return } @@ -147,7 +147,7 @@ func getDevContainer(ctx context.Context, apiClient *api.API, codespace *api.Cod var container devContainer if err := json.Unmarshal(convertedJSON, &container); err != nil { - ch <- devContainerResult{nil, fmt.Errorf("error unmarshaling: %v", err)} + ch <- devContainerResult{nil, fmt.Errorf("error unmarshaling: %w", err)} return } @@ -168,7 +168,7 @@ func newPortsPublicCmd() *cobra.Command { // should only happen if flag is not defined // or if the flag is not of string type // since it's a persistent flag that we control it should never happen - return fmt.Errorf("get codespace flag: %v", err) + return fmt.Errorf("get codespace flag: %w", err) } log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -189,7 +189,7 @@ func newPortsPrivateCmd() *cobra.Command { // should only happen if flag is not defined // or if the flag is not of string type // since it's a persistent flag that we control it should never happen - return fmt.Errorf("get codespace flag: %v", err) + return fmt.Errorf("get codespace flag: %w", err) } log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -204,7 +204,7 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) @@ -212,21 +212,21 @@ func updatePortVisibility(log *output.Logger, codespaceName, sourcePort string, if err == errNoCodespaces { return err } - return fmt.Errorf("error getting codespace: %v", err) + return fmt.Errorf("error getting codespace: %w", err) } session, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("error connecting to Live Share: %v", err) + return fmt.Errorf("error connecting to Live Share: %w", err) } port, err := strconv.Atoi(sourcePort) if err != nil { - return fmt.Errorf("error reading port number: %v", err) + return fmt.Errorf("error reading port number: %w", err) } if err := session.UpdateSharedVisibility(ctx, port, public); err != nil { - return fmt.Errorf("error update port to public: %v", err) + return fmt.Errorf("error update port to public: %w", err) } state := "PUBLIC" @@ -251,7 +251,7 @@ func newPortsForwardCmd() *cobra.Command { // should only happen if flag is not defined // or if the flag is not of string type // since it's a persistent flag that we control it should never happen - return fmt.Errorf("get codespace flag: %v", err) + return fmt.Errorf("get codespace flag: %w", err) } log := output.NewLogger(os.Stdout, os.Stderr, false) @@ -266,12 +266,12 @@ func forwardPorts(log *output.Logger, codespaceName string, ports []string) erro portPairs, err := getPortPairs(ports) if err != nil { - return fmt.Errorf("get port pairs: %v", err) + return fmt.Errorf("get port pairs: %w", err) } user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) @@ -279,12 +279,12 @@ func forwardPorts(log *output.Logger, codespaceName string, ports []string) erro if err == errNoCodespaces { return err } - return fmt.Errorf("error getting codespace: %v", err) + return fmt.Errorf("error getting codespace: %w", err) } session, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("error connecting to Live Share: %v", err) + return fmt.Errorf("error connecting to Live Share: %w", err) } // Run forwarding of all ports concurrently, aborting all of @@ -323,12 +323,12 @@ func getPortPairs(ports []string) ([]portPair, error) { remote, err := strconv.Atoi(parts[0]) if err != nil { - return pp, fmt.Errorf("convert remote port to int: %v", err) + return pp, fmt.Errorf("convert remote port to int: %w", err) } local, err := strconv.Atoi(parts[1]) if err != nil { - return pp, fmt.Errorf("convert local port to int: %v", err) + return pp, fmt.Errorf("convert local port to int: %w", err) } pp = append(pp, portPair{remote, local}) diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index e5289e205..fd2086bcc 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -46,22 +46,22 @@ func ssh(ctx context.Context, sshProfile, codespaceName string, localSSHServerPo user, err := apiClient.GetUser(ctx) if err != nil { - return fmt.Errorf("error getting user: %v", err) + return fmt.Errorf("error getting user: %w", err) } codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { - return fmt.Errorf("get or choose codespace: %v", err) + return fmt.Errorf("get or choose codespace: %w", err) } session, err := codespaces.ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("error connecting to Live Share: %v", err) + return fmt.Errorf("error connecting to Live Share: %w", err) } remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) if err != nil { - return fmt.Errorf("error getting ssh server details: %v", err) + return fmt.Errorf("error getting ssh server details: %w", err) } usingCustomPort := localSSHServerPort != 0 // suppress log of command line in Shell @@ -93,10 +93,10 @@ func ssh(ctx context.Context, sshProfile, codespaceName string, localSSHServerPo select { case err := <-tunnelClosed: - return fmt.Errorf("tunnel closed: %v", err) + return fmt.Errorf("tunnel closed: %w", err) case err := <-shellClosed: if err != nil { - return fmt.Errorf("shell closed: %v", err) + return fmt.Errorf("shell closed: %w", err) } return nil // success } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 9aee3564c..805eb3c96 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -29,7 +29,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use startedCodespace = true log.Print("Starting your codespace...") if err := apiClient.StartCodespace(ctx, token, codespace); err != nil { - return nil, fmt.Errorf("error starting codespace: %v", err) + return nil, fmt.Errorf("error starting codespace: %w", err) } } @@ -49,7 +49,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use var err error codespace, err = apiClient.GetCodespace(ctx, token, userLogin, codespace.Name) if err != nil { - return nil, fmt.Errorf("error getting codespace: %v", err) + return nil, fmt.Errorf("error getting codespace: %w", err) } } @@ -68,7 +68,7 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use }), ) if err != nil { - return nil, fmt.Errorf("error creating Live Share client: %v", err) + return nil, fmt.Errorf("error creating Live Share client: %w", err) } return lsclient.JoinWorkspace(ctx) diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go index 14dbfbb88..256acee2b 100644 --- a/internal/codespaces/ssh.go +++ b/internal/codespaces/ssh.go @@ -21,7 +21,7 @@ func StartSSHServer(ctx context.Context, session *liveshare.Session, log logger) sshServerStartResult, err := sshServer.StartRemoteServer(ctx) if err != nil { - return 0, "", fmt.Errorf("error starting live share: %v", err) + return 0, "", fmt.Errorf("error starting live share: %w", err) } if !sshServerStartResult.Result { @@ -30,7 +30,7 @@ func StartSSHServer(ctx context.Context, session *liveshare.Session, log logger) portInt, err := strconv.Atoi(sshServerStartResult.ServerPort) if err != nil { - return 0, "", fmt.Errorf("error parsing port: %v", err) + return 0, "", fmt.Errorf("error parsing port: %w", err) } return portInt, sshServerStartResult.User, nil diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 492ce3964..2d7da9d75 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -39,12 +39,12 @@ type PostCreateState struct { func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, user *api.User, codespace *api.Codespace, poller func([]PostCreateState)) error { token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) if err != nil { - return fmt.Errorf("getting codespace token: %v", err) + return fmt.Errorf("getting codespace token: %w", err) } session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("connect to Live Share: %v", err) + return fmt.Errorf("connect to Live Share: %w", err) } // Ensure local port is listening before client (getPostCreateOutput) connects. @@ -56,7 +56,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u remoteSSHServerPort, sshUser, err := StartSSHServer(ctx, session, log) if err != nil { - return fmt.Errorf("error getting ssh server details: %v", err) + return fmt.Errorf("error getting ssh server details: %w", err) } tunnelClosed := make(chan error, 1) // buffered to avoid sender stuckness @@ -74,12 +74,12 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u return ctx.Err() case err := <-tunnelClosed: - return fmt.Errorf("connection failed: %v", err) + return fmt.Errorf("connection failed: %w", err) case <-t.C: states, err := getPostCreateOutput(ctx, localPort, codespace, sshUser) if err != nil { - return fmt.Errorf("get post create output: %v", err) + return fmt.Errorf("get post create output: %w", err) } poller(states) @@ -95,13 +95,13 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod stdout := new(bytes.Buffer) cmd.Stdout = stdout if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("run command: %v", err) + return nil, fmt.Errorf("run command: %w", err) } var output struct { Steps []PostCreateState `json:"steps"` } if err := json.Unmarshal(stdout.Bytes(), &output); err != nil { - return nil, fmt.Errorf("unmarshal output: %v", err) + return nil, fmt.Errorf("unmarshal output: %w", err) } return output.Steps, nil From 547c62922050ad98fa8a20f42c2532b05c932909 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 15 Sep 2021 10:38:19 -0400 Subject: [PATCH 2/8] fix ctx cancellation errors & fix todo for X11 forwarding --- cmd/ghcs/create.go | 7 ++++++- internal/codespaces/ssh.go | 9 +++++++-- internal/codespaces/states.go | 14 +++++++------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 093450e7d..3669d7434 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -153,7 +153,12 @@ func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, use } } - if err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller); err != nil { + err := codespaces.PollPostCreateStates(ctx, log, apiClient, user, codespace, poller) + if err != nil { + if errors.Is(err, context.Canceled) && breakNextState { + return nil // we cancelled the context to stop polling, we can ignore the error + } + return fmt.Errorf("failed to poll state changes from codespace: %v", err) } diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go index 14dbfbb88..7dcab3de4 100644 --- a/internal/codespaces/ssh.go +++ b/internal/codespaces/ssh.go @@ -60,9 +60,14 @@ func NewRemoteCommand(ctx context.Context, tunnelPort int, destination, command // an interactive shell) over ssh. func newSSHCommand(ctx context.Context, port int, dst, command string) (*exec.Cmd, []string) { connArgs := []string{"-p", strconv.Itoa(port), "-o", "NoHostAuthenticationForLocalhost=yes"} - // TODO(adonovan): eliminate X11 and X11Trust flags where unneeded. - cmdArgs := append([]string{dst, "-X", "-Y", "-C"}, connArgs...) // X11, X11Trust, Compression + cmdArgs := []string{dst, "-C"} // Always use Compression + if command == "" { + // if we are in a shell send X11 and X11Trust + cmdArgs = append(cmdArgs, "-X", "-Y") + } + + cmdArgs = append(cmdArgs, connArgs...) if command != "" { cmdArgs = append(cmdArgs, command) } diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 492ce3964..2d7da9d75 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -39,12 +39,12 @@ type PostCreateState struct { func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, user *api.User, codespace *api.Codespace, poller func([]PostCreateState)) error { token, err := apiClient.GetCodespaceToken(ctx, user.Login, codespace.Name) if err != nil { - return fmt.Errorf("getting codespace token: %v", err) + return fmt.Errorf("getting codespace token: %w", err) } session, err := ConnectToLiveshare(ctx, log, apiClient, user.Login, token, codespace) if err != nil { - return fmt.Errorf("connect to Live Share: %v", err) + return fmt.Errorf("connect to Live Share: %w", err) } // Ensure local port is listening before client (getPostCreateOutput) connects. @@ -56,7 +56,7 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u remoteSSHServerPort, sshUser, err := StartSSHServer(ctx, session, log) if err != nil { - return fmt.Errorf("error getting ssh server details: %v", err) + return fmt.Errorf("error getting ssh server details: %w", err) } tunnelClosed := make(chan error, 1) // buffered to avoid sender stuckness @@ -74,12 +74,12 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u return ctx.Err() case err := <-tunnelClosed: - return fmt.Errorf("connection failed: %v", err) + return fmt.Errorf("connection failed: %w", err) case <-t.C: states, err := getPostCreateOutput(ctx, localPort, codespace, sshUser) if err != nil { - return fmt.Errorf("get post create output: %v", err) + return fmt.Errorf("get post create output: %w", err) } poller(states) @@ -95,13 +95,13 @@ func getPostCreateOutput(ctx context.Context, tunnelPort int, codespace *api.Cod stdout := new(bytes.Buffer) cmd.Stdout = stdout if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("run command: %v", err) + return nil, fmt.Errorf("run command: %w", err) } var output struct { Steps []PostCreateState `json:"steps"` } if err := json.Unmarshal(stdout.Bytes(), &output); err != nil { - return nil, fmt.Errorf("unmarshal output: %v", err) + return nil, fmt.Errorf("unmarshal output: %w", err) } return output.Steps, nil From 06719866c95e834d87fd2ecd87b5889b14d6df2b Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 15 Sep 2021 13:09:31 -0400 Subject: [PATCH 3/8] move api to internal/ --- cmd/ghcs/code.go | 2 +- cmd/ghcs/common.go | 2 +- cmd/ghcs/create.go | 2 +- cmd/ghcs/delete.go | 2 +- cmd/ghcs/list.go | 2 +- cmd/ghcs/logs.go | 2 +- cmd/ghcs/ports.go | 2 +- cmd/ghcs/ssh.go | 2 +- {api => internal/api}/api.go | 19 ++++++++++++++++++- internal/codespaces/codespaces.go | 2 +- internal/codespaces/states.go | 2 +- 11 files changed, 28 insertions(+), 11 deletions(-) rename {api => internal/api}/api.go (94%) diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 3bd67053d..0ec363a6d 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -6,7 +6,7 @@ import ( "net/url" "os" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/skratchdot/open-golang/open" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go index bfcb67496..133e6d1de 100644 --- a/cmd/ghcs/common.go +++ b/cmd/ghcs/common.go @@ -11,7 +11,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/terminal" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "golang.org/x/term" ) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 093450e7d..82414f80e 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -9,8 +9,8 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/fatih/camelcase" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 7800a13c0..3f5d68684 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -7,8 +7,8 @@ import ( "os" "strings" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index ee26e3013..dee1b0875 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -5,8 +5,8 @@ import ( "fmt" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" ) diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index a685a364d..8d7dc475e 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -6,8 +6,8 @@ import ( "net" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/spf13/cobra" diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index f6d1f6e2a..8256c13e2 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -11,8 +11,8 @@ import ( "strconv" "strings" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/muhammadmuzzammil1998/jsonc" diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index e5289e205..b020a799e 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -6,8 +6,8 @@ import ( "net" "os" - "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" + "github.com/github/ghcs/internal/api" "github.com/github/ghcs/internal/codespaces" "github.com/github/go-liveshare" "github.com/spf13/cobra" diff --git a/api/api.go b/internal/api/api.go similarity index 94% rename from api/api.go rename to internal/api/api.go index ad69f23fc..8277e903a 100644 --- a/api/api.go +++ b/internal/api/api.go @@ -1,4 +1,3 @@ -// TODO(adonovan): rename to package codespaces, and codespaces.Client. package api // For descriptions of service interfaces, see: @@ -7,6 +6,24 @@ package api // - https://github.com/github/github/blob/master/app/api/codespaces.rb (for vscs_internal) // TODO(adonovan): replace the last link with a public doc URL when available. +// TODO(adonovan): a possible reorganization would be to split this +// file into three internal packages, one per backend service, and to +// rename api.API to github.Client: +// +// - github.GetUser(github.Client) +// - github.GetRepository(Client) +// - github.ReadFile(Client, nwo, branch, path) // was GetCodespaceRepositoryContents +// - codespaces.Create(Client, user, repo, sku, branch, location) +// - codespaces.Delete(Client, user, token, name) +// - codespaces.Get(Client, token, owner, name) +// - codespaces.GetMachineTypes(Client, user, repo, branch, location) +// - codespaces.GetToken(Client, login, name) +// - codespaces.List(Client, user) +// - codespaces.Start(Client, token, codespace) +// - visualstudio.GetRegionLocation(http.Client) // no dependency on github +// +// This would make the meaning of each operation clearer. + import ( "bytes" "context" diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index 9aee3564c..804c2dab5 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -6,7 +6,7 @@ import ( "fmt" "time" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/github/go-liveshare" ) diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 492ce3964..c7a7767ae 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/github/ghcs/api" + "github.com/github/ghcs/internal/api" "github.com/github/go-liveshare" ) From 0f72e3d88642e36a1f3cbb2ef95866b2269541d2 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 15 Sep 2021 14:29:16 -0400 Subject: [PATCH 4/8] defer stopPolling and docs --- cmd/ghcs/create.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index 3669d7434..aa4c14658 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -105,12 +105,16 @@ func create(opts *createOptions) error { return nil } +// showStatus polls the codespace for a list of post create states and their status. It will keep polling +// until all states have finished. Once all states have finished, we poll once more to check if any new +// states have been introduced and stop polling otherwise. func showStatus(ctx context.Context, log *output.Logger, apiClient *api.API, user *api.User, codespace *api.Codespace) error { var lastState codespaces.PostCreateState var breakNextState bool finishedStates := make(map[string]bool) ctx, stopPolling := context.WithCancel(ctx) + defer stopPolling() poller := func(states []codespaces.PostCreateState) { var inProgress bool From ecd0c7056798bcd1b0d1fb2d65d3da7b1477a7be Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 15 Sep 2021 15:15:28 -0400 Subject: [PATCH 5/8] upgrade to go-liveshare 0.16.0 --- cmd/ghcs/ssh.go | 3 ++- internal/codespaces/ssh.go | 42 ----------------------------------- internal/codespaces/states.go | 3 ++- 3 files changed, 4 insertions(+), 44 deletions(-) diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index e5289e205..08c3bd7ca 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -59,7 +59,8 @@ func ssh(ctx context.Context, sshProfile, codespaceName string, localSSHServerPo return fmt.Errorf("error connecting to Live Share: %v", err) } - remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { return fmt.Errorf("error getting ssh server details: %v", err) } diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go index 14dbfbb88..28c2761b1 100644 --- a/internal/codespaces/ssh.go +++ b/internal/codespaces/ssh.go @@ -2,53 +2,11 @@ package codespaces import ( "context" - "errors" - "fmt" "os" "os/exec" "strconv" - "strings" - - "github.com/github/go-liveshare" ) -// StartSSHServer installs (if necessary) and starts the SSH in the codespace. -// It returns the remote port where it is running, the user to log in with, or an error if something failed. -func StartSSHServer(ctx context.Context, session *liveshare.Session, log logger) (serverPort int, user string, err error) { - log.Println("Fetching SSH details...") - - sshServer := session.SSHServer() - - sshServerStartResult, err := sshServer.StartRemoteServer(ctx) - if err != nil { - return 0, "", fmt.Errorf("error starting live share: %v", err) - } - - if !sshServerStartResult.Result { - return 0, "", errors.New(sshServerStartResult.Message) - } - - portInt, err := strconv.Atoi(sshServerStartResult.ServerPort) - if err != nil { - return 0, "", fmt.Errorf("error parsing port: %v", err) - } - - return portInt, sshServerStartResult.User, nil -} - -// Shell runs an interactive secure shell over an existing -// port-forwarding session. It runs until the shell is terminated -// (including by cancellation of the context). -func Shell(ctx context.Context, log logger, port int, destination string, usingCustomPort bool) error { - cmd, connArgs := newSSHCommand(ctx, port, destination, "") - - if usingCustomPort { - log.Println("Connection Details: ssh " + destination + " " + strings.Join(connArgs, " ")) - } - - return cmd.Run() -} - // NewRemoteCommand returns an exec.Cmd that will securely run a shell // command on the remote machine. func NewRemoteCommand(ctx context.Context, tunnelPort int, destination, command string) *exec.Cmd { diff --git a/internal/codespaces/states.go b/internal/codespaces/states.go index 492ce3964..f4375fa5d 100644 --- a/internal/codespaces/states.go +++ b/internal/codespaces/states.go @@ -54,7 +54,8 @@ func PollPostCreateStates(ctx context.Context, log logger, apiClient *api.API, u } localPort := listen.Addr().(*net.TCPAddr).Port - remoteSSHServerPort, sshUser, err := StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { return fmt.Errorf("error getting ssh server details: %v", err) } From 26d3199082dae4dcf8bf75d234e94ff781b872c4 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 15 Sep 2021 15:18:54 -0400 Subject: [PATCH 6/8] add back codespaces.Shell --- internal/codespaces/ssh.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go index 28c2761b1..d39fc17a0 100644 --- a/internal/codespaces/ssh.go +++ b/internal/codespaces/ssh.go @@ -5,8 +5,22 @@ import ( "os" "os/exec" "strconv" + "strings" ) +// Shell runs an interactive secure shell over an existing +// port-forwarding session. It runs until the shell is terminated +// (including by cancellation of the context). +func Shell(ctx context.Context, log logger, port int, destination string, usingCustomPort bool) error { + cmd, connArgs := newSSHCommand(ctx, port, destination, "") + + if usingCustomPort { + log.Println("Connection Details: ssh " + destination + " " + strings.Join(connArgs, " ")) + } + + return cmd.Run() +} + // NewRemoteCommand returns an exec.Cmd that will securely run a shell // command on the remote machine. func NewRemoteCommand(ctx context.Context, tunnelPort int, destination, command string) *exec.Cmd { From b2234969e4f728e049a9dc0f5eeff856baf91af7 Mon Sep 17 00:00:00 2001 From: Jose Garcia Date: Wed, 15 Sep 2021 15:40:07 -0400 Subject: [PATCH 7/8] update logs --- cmd/ghcs/logs.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index a685a364d..be283818f 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -70,7 +70,8 @@ func logs(ctx context.Context, log *output.Logger, codespaceName string, follow defer listen.Close() localPort := listen.Addr().(*net.TCPAddr).Port - remoteSSHServerPort, sshUser, err := codespaces.StartSSHServer(ctx, session, log) + log.Println("Fetching SSH Details...") + remoteSSHServerPort, sshUser, err := session.StartSSHServer(ctx) if err != nil { return fmt.Errorf("error getting ssh server details: %v", err) } From cc1b86461e457a5ab0eac363f360791389f1fe83 Mon Sep 17 00:00:00 2001 From: Christian Gregg Date: Thu, 16 Sep 2021 13:47:15 +0100 Subject: [PATCH 8/8] Confirm deletion of codespaces with unpushed/uncommited changes (#129) Adds a confirmation dialog on `ghcs delete` if the codespace in question has unpushed or uncommited changes. This confirmation can be skipped using the `--force` or `-f` flag. Closes: #84 Closes: #10 --- cmd/ghcs/delete.go | 72 +++++++++++++++++++++++++++++++++++++++++----- cmd/ghcs/list.go | 13 ++++----- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index b2547e350..961310675 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -7,6 +7,7 @@ import ( "os" "strings" + "github.com/AlecAivazis/survey/v2" "github.com/github/ghcs/cmd/ghcs/output" "github.com/github/ghcs/internal/api" "github.com/spf13/cobra" @@ -17,10 +18,10 @@ func newDeleteCmd() *cobra.Command { codespace string allCodespaces bool repo string + force bool ) log := output.NewLogger(os.Stdout, os.Stderr, false) - deleteCmd := &cobra.Command{ Use: "delete", Short: "Delete a codespace", @@ -29,11 +30,11 @@ func newDeleteCmd() *cobra.Command { case allCodespaces && repo != "": return errors.New("both --all and --repo is not supported.") case allCodespaces: - return deleteAll(log) + return deleteAll(log, force) case repo != "": - return deleteByRepo(log, repo) + return deleteByRepo(log, repo, force) default: - return delete_(log, codespace) + return delete_(log, codespace, force) } }, } @@ -41,6 +42,7 @@ func newDeleteCmd() *cobra.Command { deleteCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace") deleteCmd.Flags().BoolVar(&allCodespaces, "all", false, "Delete all codespaces") deleteCmd.Flags().StringVarP(&repo, "repo", "r", "", "Delete all codespaces for a repository") + deleteCmd.Flags().BoolVarP(&force, "force", "f", false, "Delete codespaces with unsaved changes without confirmation") return deleteCmd } @@ -49,7 +51,7 @@ func init() { rootCmd.AddCommand(newDeleteCmd()) } -func delete_(log *output.Logger, codespaceName string) error { +func delete_(log *output.Logger, codespaceName string, force bool) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -63,6 +65,15 @@ func delete_(log *output.Logger, codespaceName string) error { return fmt.Errorf("get or choose codespace: %w", err) } + confirmed, err := confirmDeletion(codespace, force) + if err != nil { + return fmt.Errorf("deletion could not be confirmed: %w", err) + } + + if !confirmed { + return nil + } + if err := apiClient.DeleteCodespace(ctx, user, token, codespace.Name); err != nil { return fmt.Errorf("error deleting codespace: %w", err) } @@ -72,7 +83,7 @@ func delete_(log *output.Logger, codespaceName string) error { return list(&listOptions{}) } -func deleteAll(log *output.Logger) error { +func deleteAll(log *output.Logger, force bool) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -87,6 +98,15 @@ func deleteAll(log *output.Logger) error { } for _, c := range codespaces { + confirmed, err := confirmDeletion(c, force) + if err != nil { + return fmt.Errorf("deletion could not be confirmed: %w", err) + } + + if !confirmed { + continue + } + token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) if err != nil { return fmt.Errorf("error getting codespace token: %w", err) @@ -102,7 +122,7 @@ func deleteAll(log *output.Logger) error { return list(&listOptions{}) } -func deleteByRepo(log *output.Logger, repo string) error { +func deleteByRepo(log *output.Logger, repo string, force bool) error { apiClient := api.New(os.Getenv("GITHUB_TOKEN")) ctx := context.Background() @@ -121,6 +141,16 @@ func deleteByRepo(log *output.Logger, repo string) error { if !strings.EqualFold(c.RepositoryNWO, repo) { continue } + + confirmed, err := confirmDeletion(c, force) + if err != nil { + return fmt.Errorf("deletion could not be confirmed: %w", err) + } + + if !confirmed { + continue + } + deleted = true token, err := apiClient.GetCodespaceToken(ctx, user.Login, c.Name) @@ -141,3 +171,31 @@ func deleteByRepo(log *output.Logger, repo string) error { return list(&listOptions{}) } + +func confirmDeletion(codespace *api.Codespace, force bool) (bool, error) { + gs := codespace.Environment.GitStatus + hasUnsavedChanges := gs.HasUncommitedChanges || gs.HasUnpushedChanges + if force || !hasUnsavedChanges { + return true, nil + } + if !hasTTY { + return false, fmt.Errorf("codespace %s has unsaved changes (use --force to override)", codespace.Name) + } + + var confirmed struct { + Confirmed bool + } + q := []*survey.Question{ + { + Name: "confirmed", + Prompt: &survey.Confirm{ + Message: fmt.Sprintf("Codespace %s has unsaved changes. OK to delete?", codespace.Name), + }, + }, + } + if err := ask(q, &confirmed); err != nil { + return false, fmt.Errorf("failed to prompt: %w", err) + } + + return confirmed.Confirmed, nil +} diff --git a/cmd/ghcs/list.go b/cmd/ghcs/list.go index 2db609d50..72a25becc 100644 --- a/cmd/ghcs/list.go +++ b/cmd/ghcs/list.go @@ -55,7 +55,7 @@ func list(opts *listOptions) error { table.Append([]string{ codespace.Name, codespace.RepositoryNWO, - branch(codespace), + codespace.Name + dirtyStar(codespace.Environment.GitStatus), codespace.Environment.State, codespace.CreatedAt, }) @@ -65,13 +65,10 @@ func list(opts *listOptions) error { return nil } -func branch(codespace *api.Codespace) string { - name := codespace.Branch - gitStatus := codespace.Environment.GitStatus - - if gitStatus.HasUncommitedChanges || gitStatus.HasUnpushedChanges { - name += "*" +func dirtyStar(status api.CodespaceEnvironmentGitStatus) string { + if status.HasUncommitedChanges || status.HasUnpushedChanges { + return "*" } - return name + return "" }