coder · creack · Sep 15, 2020 · Sep 13, 2020 · Sep 4, 2020 · Sep 15, 2020
diff --git a/ci/integration/setup_test.go b/ci/integration/setup_test.go
@@ -32,7 +32,7 @@ func init() {
 
 // build the coder-cli binary and move to the integration testing bin directory
 func build(path string) error {
-	tar := fmt.Sprintf("coder-cli-linux-amd64.tar.gz")
+	tar := "coder-cli-linux-amd64.tar.gz"
 	dir := filepath.Dir(path)
 	cmd := exec.Command(
 		"sh", "-c",

diff --git a/cmd/coder/main.go b/cmd/coder/main.go
@@ -15,14 +15,14 @@ import (
 	"go.coder.com/flog"
 )
 
-var (
-	version string = "unknown"
-)
+// Using a global for the version so it can be set at build time using ldflags.
+var version = "unknown"
 
 func main() {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
+	// If requested, spin up the pprof webserver.
 	if os.Getenv("PPROF") != "" {
 		go func() {
 			log.Println(http.ListenAndServe("localhost:6060", nil))
@@ -31,15 +31,19 @@ func main() {
 
 	stdoutState, err := xterminal.MakeOutputRaw(os.Stdout.Fd())
 	if err != nil {
-		flog.Fatal("set output to raw: %v", err)
+		flog.Fatal("set output to raw: %s", err)
 	}
-	defer xterminal.Restore(os.Stdout.Fd(), stdoutState)
+	defer func() {
+		// Best effort. Would result in broken terminal on window but nothing we can do about it.
+		_ = xterminal.Restore(os.Stdout.Fd(), stdoutState)
+	}()
 
 	app := cmd.Make()
 	app.Version = fmt.Sprintf("%s %s %s/%s", version, runtime.Version(), runtime.GOOS, runtime.GOARCH)
 
-	err = app.ExecuteContext(ctx)
-	if err != nil {
+	if err := app.ExecuteContext(ctx); err != nil {
+		// NOTE: The returned error is already handled and logged by the cmd lib (cobra), so no need to re-handle it here.
+		//       As we are in the main, if there was an error, exit the process with an error code.
 		os.Exit(1)
 	}
 }
diff --git a/coder-sdk/activity.go b/coder-sdk/activity.go
@@ -5,18 +5,23 @@ import (
 	"net/http"
 )
 
+type activityRequest struct {
+	Source        string `json:"source"`
+	EnvironmentID string `json:"environment_id"`
+}
+
 // PushActivity pushes CLI activity to Coder.
-func (c Client) PushActivity(ctx context.Context, source string, envID string) error {
-	res, err := c.request(ctx, http.MethodPost, "/api/metrics/usage/push", map[string]string{
-		"source":         source,
-		"environment_id": envID,
+func (c Client) PushActivity(ctx context.Context, source, envID string) error {
+	resp, err := c.request(ctx, http.MethodPost, "/api/metrics/usage/push", activityRequest{
+		Source:        source,
+		EnvironmentID: envID,
 	})
 	if err != nil {
 		return err
 	}
 
-	if res.StatusCode != http.StatusOK {
-		return bodyError(res)
+	if resp.StatusCode != http.StatusOK {
+		return bodyError(resp)
 	}
 	return nil
 }
diff --git a/coder-sdk/client.go b/coder-sdk/client.go
@@ -15,12 +15,17 @@ type Client struct {
 	Token   string
 }
 
-func (c Client) copyURL() *url.URL {
-	swp := *c.BaseURL
-	return &swp
-}
-
-func (c *Client) http() (*http.Client, error) {
+// newHTTPClient creates a default underlying http client and sets the auth cookie.
+//
+// NOTE: As we do not specify a custom transport, the default one from the stdlib will be used,
+//       resulting in a persistent connection pool.
+//       We do not set a timeout here as it could cause issue with the websocket.
+//       The caller is expected to set it when needed.
+//
+// WARNING: If the caller sets a custom transport to set TLS settings or a custom CA, the default
+//          pool will not be used and it might result in a new dns lookup/tls session/socket begin
+//          established each time.
+func (c *Client) newHTTPClient() (*http.Client, error) {
 	jar, err := cookiejar.New(nil)
 	if err != nil {
 		return nil, err
@@ -36,5 +41,6 @@ func (c *Client) http() (*http.Client, error) {
 			Secure:   c.BaseURL.Scheme == "https",
 		},
 	})
+
 	return &http.Client{Jar: jar}, nil
 }
diff --git a/coder-sdk/devurl.go b/coder-sdk/devurl.go
@@ -6,7 +6,7 @@ import (
 	"net/http"
 )
 
-// DevURL is the parsed json response record for a devURL from cemanager
+// DevURL is the parsed json response record for a devURL from cemanager.
 type DevURL struct {
 	ID     string `json:"id"     tab:"ID"`
 	URL    string `json:"url"    tab:"URL"`
@@ -20,21 +20,21 @@ type delDevURLRequest struct {
 	DevURLID string `json:"url_id"`
 }
 
-// DelDevURL deletes the specified devurl
+// DelDevURL deletes the specified devurl.
 func (c Client) DelDevURL(ctx context.Context, envID, urlID string) error {
 	reqURL := fmt.Sprintf("/api/environments/%s/devurls/%s", envID, urlID)
 
-	res, err := c.request(ctx, http.MethodDelete, reqURL, delDevURLRequest{
+	resp, err := c.request(ctx, http.MethodDelete, reqURL, delDevURLRequest{
 		EnvID:    envID,
 		DevURLID: urlID,
 	})
 	if err != nil {
 		return err
 	}
-	defer res.Body.Close()
+	defer func() { _ = resp.Body.Close() }() // Best effort. Likely connection drop.
 
-	if res.StatusCode != http.StatusOK {
-		return bodyError(res)
+	if resp.StatusCode != http.StatusOK {
+		return bodyError(resp)
 	}
 
 	return nil
@@ -47,11 +47,11 @@ type createDevURLRequest struct {
 	Name   string `json:"name"`
 }
 
-// InsertDevURL inserts a new devurl for the authenticated user
+// InsertDevURL inserts a new devurl for the authenticated user.
 func (c Client) InsertDevURL(ctx context.Context, envID string, port int, name, access string) error {
 	reqURL := fmt.Sprintf("/api/environments/%s/devurls", envID)
 
-	res, err := c.request(ctx, http.MethodPost, reqURL, createDevURLRequest{
+	resp, err := c.request(ctx, http.MethodPost, reqURL, createDevURLRequest{
 		EnvID:  envID,
 		Port:   port,
 		Access: access,
@@ -60,22 +60,22 @@ func (c Client) InsertDevURL(ctx context.Context, envID string, port int, name,
 	if err != nil {
 		return err
 	}
-	defer res.Body.Close()
+	defer func() { _ = resp.Body.Close() }() // Best effort. Likely connection drop.
 
-	if res.StatusCode != http.StatusOK {
-		return bodyError(res)
+	if resp.StatusCode != http.StatusOK {
+		return bodyError(resp)
 	}
 
 	return nil
 }
 
 type updateDevURLRequest createDevURLRequest
 
-// UpdateDevURL updates an existing devurl for the authenticated user
+// UpdateDevURL updates an existing devurl for the authenticated user.
 func (c Client) UpdateDevURL(ctx context.Context, envID, urlID string, port int, name, access string) error {
 	reqURL := fmt.Sprintf("/api/environments/%s/devurls/%s", envID, urlID)
 
-	res, err := c.request(ctx, http.MethodPut, reqURL, updateDevURLRequest{
+	resp, err := c.request(ctx, http.MethodPut, reqURL, updateDevURLRequest{
 		EnvID:  envID,
 		Port:   port,
 		Access: access,
@@ -84,10 +84,10 @@ func (c Client) UpdateDevURL(ctx context.Context, envID, urlID string, port int,
 	if err != nil {
 		return err
 	}
-	defer res.Body.Close()
+	defer func() { _ = resp.Body.Close() }() // Best effort. Likefly connection drop.
 
-	if res.StatusCode != http.StatusOK {
-		return bodyError(res)
+	if resp.StatusCode != http.StatusOK {
+		return bodyError(resp)
 	}
 
 	return nil

diff --git a/coder-sdk/env.go b/coder-sdk/env.go
@@ -31,13 +31,18 @@ type Environment struct {
 	UpdatedAt        time.Time        `json:"updated_at"         tab:"-"`
 	LastOpenedAt     time.Time        `json:"last_opened_at"     tab:"-"`
 	LastConnectionAt time.Time        `json:"last_connection_at" tab:"-"`
-	AutoOffThreshold xjson.Duration   `json:"auto_off_threshold" tab:"-"`
+	AutoOffThreshold xjson.MSDuration `json:"auto_off_threshold" tab:"-"`
 }
 
 // RebuildMessage defines the message shown when an Environment requires a rebuild for it can be accessed.
 type RebuildMessage struct {
-	Text     string `json:"text"`
-	Required bool   `json:"required"`
+	Text             string           `json:"text"`
+	Required         bool             `json:"required"`
+	AutoOffThreshold xjson.MSDuration `json:"auto_off_threshold" tab:"-"`
+	RebuildMessages  []struct {
+		Text     string `json:"text"`
+		Required bool   `json:"required"`
+	} `json:"rebuild_messages" tab:"-"`
 }
 
 // EnvironmentStat represents the state of an environment
@@ -53,9 +58,7 @@ type EnvironmentStat struct {
 	DiskUsed        int64             `json:"disk_used"`
 }
 
-func (e EnvironmentStat) String() string {
-	return string(e.ContainerStatus)
-}
+func (e EnvironmentStat) String() string { return string(e.ContainerStatus) }
 
 // EnvironmentStatus refers to the states of an environment.
 type EnvironmentStatus string
@@ -69,7 +72,7 @@ const (
 	EnvironmentUnknown  EnvironmentStatus = "UNKNOWN"
 )
 
-// CreateEnvironmentRequest is used to configure a new environment
+// CreateEnvironmentRequest is used to configure a new environment.
 type CreateEnvironmentRequest struct {
 	Name     string   `json:"name"`
 	ImageID  string   `json:"image_id"`
@@ -84,61 +87,50 @@ type CreateEnvironmentRequest struct {
 // CreateEnvironment sends a request to create an environment.
 func (c Client) CreateEnvironment(ctx context.Context, orgID string, req CreateEnvironmentRequest) (*Environment, error) {
 	var env Environment
-	err := c.requestBody(
-		ctx,
-		http.MethodPost, "/api/orgs/"+orgID+"/environments",
-		req,
-		&env,
-	)
-	return &env, err
+	if err := c.requestBody(ctx, http.MethodPost, "/api/orgs/"+orgID+"/environments", req, &env); err != nil {
+		return nil, err
+	}
+	return &env, nil
 }
 
 // EnvironmentsByOrganization gets the list of environments owned by the given user.
 func (c Client) EnvironmentsByOrganization(ctx context.Context, userID, orgID string) ([]Environment, error) {
 	var envs []Environment
-	err := c.requestBody(
-		ctx,
-		http.MethodGet, "/api/orgs/"+orgID+"/members/"+userID+"/environments",
-		nil,
-		&envs,
-	)
-	return envs, err
+	if err := c.requestBody(ctx, http.MethodGet, "/api/orgs/"+orgID+"/members/"+userID+"/environments", nil, &envs); err != nil {
+		return nil, err
+	}
+	return envs, nil
 }
 
 // DeleteEnvironment deletes the environment.
 func (c Client) DeleteEnvironment(ctx context.Context, envID string) error {
-	return c.requestBody(
-		ctx,
-		http.MethodDelete, "/api/environments/"+envID,
-		nil,
-		nil,
-	)
+	return c.requestBody(ctx, http.MethodDelete, "/api/environments/"+envID, nil, nil)
 }
 
 // DialWsep dials an environments command execution interface
-// See github.com/cdr/wsep for details
+// See https://github.com/cdr/wsep for details.
 func (c Client) DialWsep(ctx context.Context, env *Environment) (*websocket.Conn, error) {
-	return c.dialWs(ctx, "/proxy/environments/"+env.ID+"/wsep")
+	return c.dialWebsocket(ctx, "/proxy/environments/"+env.ID+"/wsep")
 }
 
 // DialIDEStatus opens a websocket connection for cpu load metrics on the environment
 func (c Client) DialIDEStatus(ctx context.Context, envID string) (*websocket.Conn, error) {
-	return c.dialWs(ctx, "/proxy/environments/"+envID+"/ide/api/status")
+	return c.dialWebsocket(ctx, "/proxy/environments/"+envID+"/ide/api/status")
 }
 
-// DialEnvironmentBuildLog opens a websocket connection for the environment build log messages
+// DialEnvironmentBuildLog opens a websocket connection for the environment build log messages.
 func (c Client) DialEnvironmentBuildLog(ctx context.Context, envID string) (*websocket.Conn, error) {
-	return c.dialWs(ctx, "/api/environments/"+envID+"/watch-update")
+	return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-update")
 }
 
-// DialEnvironmentStats opens a websocket connection for environment stats
+// DialEnvironmentStats opens a websocket connection for environment stats.
 func (c Client) DialEnvironmentStats(ctx context.Context, envID string) (*websocket.Conn, error) {
-	return c.dialWs(ctx, "/api/environments/"+envID+"/watch-stats")
+	return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-stats")
 }
 
 // DialResourceLoad opens a websocket connection for cpu load metrics on the environment
 func (c Client) DialResourceLoad(ctx context.Context, envID string) (*websocket.Conn, error) {
-	return c.dialWs(ctx, "/api/environments/"+envID+"/watch-resource-load")
+	return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-resource-load")
 }
 
 // BuildLogType describes the type of an event.

diff --git a/coder-sdk/error.go b/coder-sdk/error.go
@@ -11,9 +11,10 @@ import (
 // ErrNotFound describes an error case in which the requested resource could not be found
 var ErrNotFound = xerrors.Errorf("resource not found")
 
+// apiError is the expected payload format for our errors.
 type apiError struct {
 	Err struct {
-		Msg string `json:"msg,required"`
+		Msg string `json:"msg"`
 	} `json:"error"`
 }
 
@@ -24,9 +25,12 @@ func bodyError(resp *http.Response) error {
 	}
 
 	var msg apiError
-	err = json.NewDecoder(resp.Body).Decode(&msg)
-	if err != nil || msg.Err.Msg == "" {
+	// Try to decode the payload as an error, if it fails or if there is no error message,
+	// return the response URL with the dump.
+	if err := json.NewDecoder(resp.Body).Decode(&msg); err != nil || msg.Err.Msg == "" {
 		return xerrors.Errorf("%s\n%s", resp.Request.URL, byt)
 	}
+
+	// If the payload was a in the expected error format with a message, include it.
 	return xerrors.Errorf("%s\n%s%s", resp.Request.URL, byt, msg.Err.Msg)
 }