Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Commit 6fb7c7b

Browse files
creackcreack
committed
Cleanup:
- Scope errors when possible. - Fix nil pointers in unmarshal. - Align struct tags for readability. - Explicitly discard errors with a comment on why. - Import package order. - Fmt verb for types. - More comments. Signed-off-by: Guillaume J. Charmes <guillaume@coder.com>
1 parent 04a201f commit 6fb7c7b

28 files changed

+438
-379
lines changed

‎ci/integration/setup_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func init() {
3232

3333
// build the coder-cli binary and move to the integration testing bin directory
3434
func build(path string) error {
35-
tar := fmt.Sprintf("coder-cli-linux-amd64.tar.gz")
35+
tar := "coder-cli-linux-amd64.tar.gz"
3636
dir := filepath.Dir(path)
3737
cmd := exec.Command(
3838
"sh", "-c",

‎cmd/coder/main.go

+11-7
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ import (
1515
"go.coder.com/flog"
1616
)
1717

18-
var (
19-
version string = "unknown"
20-
)
18+
// Using a global for the version so it can be set at build time using ldflags.
19+
var version = "unknown"
2120

2221
func main() {
2322
ctx, cancel := context.WithCancel(context.Background())
2423
defer cancel()
2524

25+
// If requested, spin up the pprof webserver.
2626
if os.Getenv("PPROF") != "" {
2727
go func() {
2828
log.Println(http.ListenAndServe("localhost:6060", nil))
@@ -31,15 +31,19 @@ func main() {
3131

3232
stdoutState, err := xterminal.MakeOutputRaw(os.Stdout.Fd())
3333
if err != nil {
34-
flog.Fatal("set output to raw: %v", err)
34+
flog.Fatal("set output to raw: %s", err)
3535
}
36-
defer xterminal.Restore(os.Stdout.Fd(), stdoutState)
36+
defer func() {
37+
// Best effort. Would result in broken terminal on window but nothing we can do about it.
38+
_ = xterminal.Restore(os.Stdout.Fd(), stdoutState)
39+
}()
3740

3841
app := cmd.Make()
3942
app.Version = fmt.Sprintf("%s %s %s/%s", version, runtime.Version(), runtime.GOOS, runtime.GOARCH)
4043

41-
err = app.ExecuteContext(ctx)
42-
if err != nil {
44+
if err := app.ExecuteContext(ctx); err != nil {
45+
// NOTE: The returned error is already handled and logged by the cmd lib (cobra), so no need to re-handle it here.
46+
// As we are in the main, if there was an error, exit the process with an error code.
4347
os.Exit(1)
4448
}
4549
}

‎coder-sdk/activity.go

+11-6
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,23 @@ import (
55
"net/http"
66
)
77

8+
type activityRequest struct {
9+
Source string `json:"source"`
10+
EnvironmentID string `json:"environment_id"`
11+
}
12+
813
// PushActivity pushes CLI activity to Coder.
9-
func (c Client) PushActivity(ctx context.Context, source string, envID string) error {
10-
res, err := c.request(ctx, http.MethodPost, "/api/metrics/usage/push", map[string]string{
11-
"source": source,
12-
"environment_id": envID,
14+
func (c Client) PushActivity(ctx context.Context, source, envID string) error {
15+
resp, err := c.request(ctx, http.MethodPost, "/api/metrics/usage/push", activityRequest{
16+
Source: source,
17+
EnvironmentID: envID,
1318
})
1419
if err != nil {
1520
return err
1621
}
1722

18-
if res.StatusCode != http.StatusOK {
19-
return bodyError(res)
23+
if resp.StatusCode != http.StatusOK {
24+
return bodyError(resp)
2025
}
2126
return nil
2227
}

‎coder-sdk/client.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,17 @@ type Client struct {
1515
Token string
1616
}
1717

18-
func (c Client) copyURL() *url.URL {
19-
swp := *c.BaseURL
20-
return &swp
21-
}
22-
23-
func (c *Client) http() (*http.Client, error) {
18+
// newHTTPClient creates a default underlying http client and sets the auth cookie.
19+
//
20+
// NOTE: As we do not specify a custom transport, the default one from the stdlib will be used,
21+
// resulting in a persistent connection pool.
22+
// We do not set a timeout here as it could cause issue with the websocket.
23+
// The caller is expected to set it when needed.
24+
//
25+
// WARNING: If the caller sets a custom transport to set TLS settings or a custom CA, the default
26+
// pool will not be used and it might result in a new dns lookup/tls session/socket begin
27+
// established each time.
28+
func (c *Client) newHTTPClient() (*http.Client, error) {
2429
jar, err := cookiejar.New(nil)
2530
if err != nil {
2631
return nil, err
@@ -36,5 +41,6 @@ func (c *Client) http() (*http.Client, error) {
3641
Secure: c.BaseURL.Scheme == "https",
3742
},
3843
})
44+
3945
return &http.Client{Jar: jar}, nil
4046
}

‎coder-sdk/devurl.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"net/http"
77
)
88

9-
// DevURL is the parsed json response record for a devURL from cemanager
9+
// DevURL is the parsed json response record for a devURL from cemanager.
1010
type DevURL struct {
1111
ID string `json:"id" tab:"ID"`
1212
URL string `json:"url" tab:"URL"`
@@ -20,21 +20,21 @@ type delDevURLRequest struct {
2020
DevURLID string `json:"url_id"`
2121
}
2222

23-
// DelDevURL deletes the specified devurl
23+
// DelDevURL deletes the specified devurl.
2424
func (c Client) DelDevURL(ctx context.Context, envID, urlID string) error {
2525
reqURL := fmt.Sprintf("/api/environments/%s/devurls/%s", envID, urlID)
2626

27-
res, err := c.request(ctx, http.MethodDelete, reqURL, delDevURLRequest{
27+
resp, err := c.request(ctx, http.MethodDelete, reqURL, delDevURLRequest{
2828
EnvID: envID,
2929
DevURLID: urlID,
3030
})
3131
if err != nil {
3232
return err
3333
}
34-
defer res.Body.Close()
34+
defer func() { _ = resp.Body.Close() }() // Best effort. Likely connection drop.
3535

36-
if res.StatusCode != http.StatusOK {
37-
return bodyError(res)
36+
if resp.StatusCode != http.StatusOK {
37+
return bodyError(resp)
3838
}
3939

4040
return nil
@@ -47,11 +47,11 @@ type createDevURLRequest struct {
4747
Name string `json:"name"`
4848
}
4949

50-
// InsertDevURL inserts a new devurl for the authenticated user
50+
// InsertDevURL inserts a new devurl for the authenticated user.
5151
func (c Client) InsertDevURL(ctx context.Context, envID string, port int, name, access string) error {
5252
reqURL := fmt.Sprintf("/api/environments/%s/devurls", envID)
5353

54-
res, err := c.request(ctx, http.MethodPost, reqURL, createDevURLRequest{
54+
resp, err := c.request(ctx, http.MethodPost, reqURL, createDevURLRequest{
5555
EnvID: envID,
5656
Port: port,
5757
Access: access,
@@ -60,22 +60,22 @@ func (c Client) InsertDevURL(ctx context.Context, envID string, port int, name,
6060
if err != nil {
6161
return err
6262
}
63-
defer res.Body.Close()
63+
defer func() { _ = resp.Body.Close() }() // Best effort. Likely connection drop.
6464

65-
if res.StatusCode != http.StatusOK {
66-
return bodyError(res)
65+
if resp.StatusCode != http.StatusOK {
66+
return bodyError(resp)
6767
}
6868

6969
return nil
7070
}
7171

7272
type updateDevURLRequest createDevURLRequest
7373

74-
// UpdateDevURL updates an existing devurl for the authenticated user
74+
// UpdateDevURL updates an existing devurl for the authenticated user.
7575
func (c Client) UpdateDevURL(ctx context.Context, envID, urlID string, port int, name, access string) error {
7676
reqURL := fmt.Sprintf("/api/environments/%s/devurls/%s", envID, urlID)
7777

78-
res, err := c.request(ctx, http.MethodPut, reqURL, updateDevURLRequest{
78+
resp, err := c.request(ctx, http.MethodPut, reqURL, updateDevURLRequest{
7979
EnvID: envID,
8080
Port: port,
8181
Access: access,
@@ -84,10 +84,10 @@ func (c Client) UpdateDevURL(ctx context.Context, envID, urlID string, port int,
8484
if err != nil {
8585
return err
8686
}
87-
defer res.Body.Close()
87+
defer func() { _ = resp.Body.Close() }() // Best effort. Likefly connection drop.
8888

89-
if res.StatusCode != http.StatusOK {
90-
return bodyError(res)
89+
if resp.StatusCode != http.StatusOK {
90+
return bodyError(resp)
9191
}
9292

9393
return nil

‎coder-sdk/env.go

+27-35
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,18 @@ type Environment struct {
3131
UpdatedAt time.Time `json:"updated_at" tab:"-"`
3232
LastOpenedAt time.Time `json:"last_opened_at" tab:"-"`
3333
LastConnectionAt time.Time `json:"last_connection_at" tab:"-"`
34-
AutoOffThreshold xjson.Duration `json:"auto_off_threshold" tab:"-"`
34+
AutoOffThreshold xjson.MSDuration `json:"auto_off_threshold" tab:"-"`
3535
}
3636

3737
// RebuildMessage defines the message shown when an Environment requires a rebuild for it can be accessed.
3838
type RebuildMessage struct {
39-
Text string `json:"text"`
40-
Required bool `json:"required"`
39+
Text string `json:"text"`
40+
Required bool `json:"required"`
41+
AutoOffThreshold xjson.MSDuration `json:"auto_off_threshold" tab:"-"`
42+
RebuildMessages []struct {
43+
Text string `json:"text"`
44+
Required bool `json:"required"`
45+
} `json:"rebuild_messages" tab:"-"`
4146
}
4247

4348
// EnvironmentStat represents the state of an environment
@@ -53,9 +58,7 @@ type EnvironmentStat struct {
5358
DiskUsed int64 `json:"disk_used"`
5459
}
5560

56-
func (e EnvironmentStat) String() string {
57-
return string(e.ContainerStatus)
58-
}
61+
func (e EnvironmentStat) String() string { return string(e.ContainerStatus) }
5962

6063
// EnvironmentStatus refers to the states of an environment.
6164
type EnvironmentStatus string
@@ -69,7 +72,7 @@ const (
6972
EnvironmentUnknown EnvironmentStatus = "UNKNOWN"
7073
)
7174

72-
// CreateEnvironmentRequest is used to configure a new environment
75+
// CreateEnvironmentRequest is used to configure a new environment.
7376
type CreateEnvironmentRequest struct {
7477
Name string `json:"name"`
7578
ImageID string `json:"image_id"`
@@ -84,61 +87,50 @@ type CreateEnvironmentRequest struct {
8487
// CreateEnvironment sends a request to create an environment.
8588
func (c Client) CreateEnvironment(ctx context.Context, orgID string, req CreateEnvironmentRequest) (*Environment, error) {
8689
var env Environment
87-
err := c.requestBody(
88-
ctx,
89-
http.MethodPost, "/api/orgs/"+orgID+"/environments",
90-
req,
91-
&env,
92-
)
93-
return &env, err
90+
if err := c.requestBody(ctx, http.MethodPost, "/api/orgs/"+orgID+"/environments", req, &env); err != nil {
91+
return nil, err
92+
}
93+
return &env, nil
9494
}
9595

9696
// EnvironmentsByOrganization gets the list of environments owned by the given user.
9797
func (c Client) EnvironmentsByOrganization(ctx context.Context, userID, orgID string) ([]Environment, error) {
9898
var envs []Environment
99-
err := c.requestBody(
100-
ctx,
101-
http.MethodGet, "/api/orgs/"+orgID+"/members/"+userID+"/environments",
102-
nil,
103-
&envs,
104-
)
105-
return envs, err
99+
if err := c.requestBody(ctx, http.MethodGet, "/api/orgs/"+orgID+"/members/"+userID+"/environments", nil, &envs); err != nil {
100+
return nil, err
101+
}
102+
return envs, nil
106103
}
107104

108105
// DeleteEnvironment deletes the environment.
109106
func (c Client) DeleteEnvironment(ctx context.Context, envID string) error {
110-
return c.requestBody(
111-
ctx,
112-
http.MethodDelete, "/api/environments/"+envID,
113-
nil,
114-
nil,
115-
)
107+
return c.requestBody(ctx, http.MethodDelete, "/api/environments/"+envID, nil, nil)
116108
}
117109

118110
// DialWsep dials an environments command execution interface
119-
// See github.com/cdr/wsep for details
111+
// See https://github.com/cdr/wsep for details.
120112
func (c Client) DialWsep(ctx context.Context, env *Environment) (*websocket.Conn, error) {
121-
return c.dialWs(ctx, "/proxy/environments/"+env.ID+"/wsep")
113+
return c.dialWebsocket(ctx, "/proxy/environments/"+env.ID+"/wsep")
122114
}
123115

124116
// DialIDEStatus opens a websocket connection for cpu load metrics on the environment
125117
func (c Client) DialIDEStatus(ctx context.Context, envID string) (*websocket.Conn, error) {
126-
return c.dialWs(ctx, "/proxy/environments/"+envID+"/ide/api/status")
118+
return c.dialWebsocket(ctx, "/proxy/environments/"+envID+"/ide/api/status")
127119
}
128120

129-
// DialEnvironmentBuildLog opens a websocket connection for the environment build log messages
121+
// DialEnvironmentBuildLog opens a websocket connection for the environment build log messages.
130122
func (c Client) DialEnvironmentBuildLog(ctx context.Context, envID string) (*websocket.Conn, error) {
131-
return c.dialWs(ctx, "/api/environments/"+envID+"/watch-update")
123+
return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-update")
132124
}
133125

134-
// DialEnvironmentStats opens a websocket connection for environment stats
126+
// DialEnvironmentStats opens a websocket connection for environment stats.
135127
func (c Client) DialEnvironmentStats(ctx context.Context, envID string) (*websocket.Conn, error) {
136-
return c.dialWs(ctx, "/api/environments/"+envID+"/watch-stats")
128+
return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-stats")
137129
}
138130

139131
// DialResourceLoad opens a websocket connection for cpu load metrics on the environment
140132
func (c Client) DialResourceLoad(ctx context.Context, envID string) (*websocket.Conn, error) {
141-
return c.dialWs(ctx, "/api/environments/"+envID+"/watch-resource-load")
133+
return c.dialWebsocket(ctx, "/api/environments/"+envID+"/watch-resource-load")
142134
}
143135

144136
// BuildLogType describes the type of an event.

‎coder-sdk/error.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
// ErrNotFound describes an error case in which the requested resource could not be found
1212
var ErrNotFound = xerrors.Errorf("resource not found")
1313

14+
// apiError is the expected payload format for our errors.
1415
type apiError struct {
1516
Err struct {
1617
Msg string `json:"msg,required"`
@@ -24,9 +25,12 @@ func bodyError(resp *http.Response) error {
2425
}
2526

2627
var msg apiError
27-
err = json.NewDecoder(resp.Body).Decode(&msg)
28-
if err != nil || msg.Err.Msg == "" {
28+
// Try to decode the payload as an error, if it fails or if there is no error message,
29+
// return the response URL with the dump.
30+
if err := json.NewDecoder(resp.Body).Decode(&msg); err != nil || msg.Err.Msg == "" {
2931
return xerrors.Errorf("%s\n%s", resp.Request.URL, byt)
3032
}
33+
34+
// If the payload was a in the expected error format with a message, include it.
3135
return xerrors.Errorf("%s\n%s%s", resp.Request.URL, byt, msg.Err.Msg)
3236
}

‎coder-sdk/image.go

+15-21
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ type Image struct {
1111
OrganizationID string `json:"organization_id"`
1212
Repository string `json:"repository"`
1313
Description string `json:"description"`
14-
URL string `json:"url"` // user-supplied URL for image
14+
URL string `json:"url"` // User-supplied URL for image.
1515
DefaultCPUCores float32 `json:"default_cpu_cores"`
1616
DefaultMemoryGB int `json:"default_memory_gb"`
1717
DefaultDiskGB int `json:"default_disk_gb"`
@@ -28,28 +28,22 @@ type NewRegistryRequest struct {
2828

2929
// ImportImageRequest is used to import new images and registries into Coder
3030
type ImportImageRequest struct {
31-
// RegistryID is used to import images to existing registries.
32-
RegistryID *string `json:"registry_id"`
33-
// NewRegistry is used when adding a new registry.
34-
NewRegistry *NewRegistryRequest `json:"new_registry"`
35-
// Repository refers to the image. For example: "codercom/ubuntu".
36-
Repository string `json:"repository"`
37-
Tag string `json:"tag"`
38-
DefaultCPUCores float32 `json:"default_cpu_cores"`
39-
DefaultMemoryGB int `json:"default_memory_gb"`
40-
DefaultDiskGB int `json:"default_disk_gb"`
41-
Description string `json:"description"`
42-
URL string `json:"url"`
31+
RegistryID *string `json:"registry_id"` // Used to import images to existing registries.
32+
NewRegistry *NewRegistryRequest `json:"new_registry"` // Used when adding a new registry.
33+
Repository string `json:"repository"` // Refers to the image. Ex: "codercom/ubuntu".
34+
Tag string `json:"tag"`
35+
DefaultCPUCores float32 `json:"default_cpu_cores"`
36+
DefaultMemoryGB int `json:"default_memory_gb"`
37+
DefaultDiskGB int `json:"default_disk_gb"`
38+
Description string `json:"description"`
39+
URL string `json:"url"`
4340
}
4441

4542
// ImportImage creates a new image and optionally a new registry
4643
func (c Client) ImportImage(ctx context.Context, orgID string, req ImportImageRequest) (*Image, error) {
47-
var img *Image
48-
err := c.requestBody(
49-
ctx,
50-
http.MethodPost, "/api/orgs/"+orgID+"/images",
51-
req,
52-
img,
53-
)
54-
return img, err
44+
var img Image
45+
if err := c.requestBody(ctx, http.MethodPost, "/api/orgs/"+orgID+"/images", req, &img); err != nil {
46+
return nil, err
47+
}
48+
return &img, nil
5549
}

0 commit comments

Comments
 (0)