Skip to content

Commit a3af9b1

Browse files
coadlercoadler
committed
fix: use *string instead of error in healthcheck response
`error`s marshal to `{}` for some reason, so this allows us to actually extract the error.
1 parent b4ca285 commit a3af9b1

File tree

8 files changed

+59
-43
lines changed

8 files changed

+59
-43
lines changed

‎coderd/healthcheck/accessurl.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ import (
88
"time"
99

1010
"golang.org/x/xerrors"
11+
12+
"github.com/coder/coder/coderd/util/ptr"
1113
)
1214

1315
type AccessURLReport struct {
14-
Healthy bool `json:"healthy"`
15-
Reachable bool `json:"reachable"`
16-
StatusCode int `json:"status_code"`
17-
HealthzResponse string `json:"healthz_response"`
18-
Error error `json:"error"`
16+
AccessURL string `json:"access_url"`
17+
Healthy bool `json:"healthy"`
18+
Reachable bool `json:"reachable"`
19+
StatusCode int `json:"status_code"`
20+
HealthzResponse string `json:"healthz_response"`
21+
Error *string `json:"error"`
1922
}
2023

2124
type AccessURLReportOptions struct {
@@ -28,36 +31,37 @@ func (r *AccessURLReport) Run(ctx context.Context, opts *AccessURLReportOptions)
2831
defer cancel()
2932

3033
if opts.AccessURL == nil {
31-
r.Error = xerrors.New("access URL is nil")
34+
r.Error = ptr.Ref("access URL is nil")
3235
return
3336
}
37+
r.AccessURL = opts.AccessURL.String()
3438

3539
if opts.Client == nil {
3640
opts.Client = http.DefaultClient
3741
}
3842

3943
accessURL, err := opts.AccessURL.Parse("/healthz")
4044
if err != nil {
41-
r.Error = xerrors.Errorf("parse healthz endpoint: %w", err)
45+
r.Error = convertError(xerrors.Errorf("parse healthz endpoint: %w", err))
4246
return
4347
}
4448

4549
req, err := http.NewRequestWithContext(ctx, "GET", accessURL.String(), nil)
4650
if err != nil {
47-
r.Error = xerrors.Errorf("create healthz request: %w", err)
51+
r.Error = convertError(xerrors.Errorf("create healthz request: %w", err))
4852
return
4953
}
5054

5155
res, err := opts.Client.Do(req)
5256
if err != nil {
53-
r.Error = xerrors.Errorf("get healthz endpoint: %w", err)
57+
r.Error = convertError(xerrors.Errorf("get healthz endpoint: %w", err))
5458
return
5559
}
5660
defer res.Body.Close()
5761

5862
body, err := io.ReadAll(res.Body)
5963
if err != nil {
60-
r.Error = xerrors.Errorf("read healthz response: %w", err)
64+
r.Error = convertError(xerrors.Errorf("read healthz response: %w", err))
6165
return
6266
}
6367

‎coderd/healthcheck/accessurl_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/coder/coder/coderd/coderdtest"
1515
"github.com/coder/coder/coderd/healthcheck"
16+
"github.com/coder/coder/coderd/util/ptr"
1617
)
1718

1819
func TestAccessURL(t *testing.T) {
@@ -36,7 +37,7 @@ func TestAccessURL(t *testing.T) {
3637
assert.True(t, report.Reachable)
3738
assert.Equal(t, http.StatusOK, report.StatusCode)
3839
assert.Equal(t, "OK", report.HealthzResponse)
39-
assert.NoError(t, report.Error)
40+
assert.Nil(t, report.Error)
4041
})
4142

4243
t.Run("404", func(t *testing.T) {
@@ -66,7 +67,7 @@ func TestAccessURL(t *testing.T) {
6667
assert.True(t, report.Reachable)
6768
assert.Equal(t, http.StatusNotFound, report.StatusCode)
6869
assert.Equal(t, string(resp), report.HealthzResponse)
69-
assert.NoError(t, report.Error)
70+
assert.Nil(t, report.Error)
7071
})
7172

7273
t.Run("ClientErr", func(t *testing.T) {
@@ -102,7 +103,7 @@ func TestAccessURL(t *testing.T) {
102103
assert.False(t, report.Reachable)
103104
assert.Equal(t, 0, report.StatusCode)
104105
assert.Equal(t, "", report.HealthzResponse)
105-
assert.ErrorIs(t, report.Error, expErr)
106+
assert.Equal(t, report.Error, ptr.Ref(expErr.Error()))
106107
})
107108
}
108109

‎coderd/healthcheck/database.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type DatabaseReport struct {
1414
Healthy bool `json:"healthy"`
1515
Reachable bool `json:"reachable"`
1616
Latency time.Duration `json:"latency"`
17-
Error error `json:"error"`
17+
Error *string `json:"error"`
1818
}
1919

2020
type DatabaseReportOptions struct {
@@ -31,7 +31,7 @@ func (r *DatabaseReport) Run(ctx context.Context, opts *DatabaseReportOptions) {
3131
for i := 0; i < pingCount; i++ {
3232
pong, err := opts.DB.Ping(ctx)
3333
if err != nil {
34-
r.Error = xerrors.Errorf("ping: %w", err)
34+
r.Error = convertError(xerrors.Errorf("ping: %w", err))
3535
return
3636
}
3737
pings = append(pings, pong)

‎coderd/healthcheck/database_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestDatabase(t *testing.T) {
3535
assert.True(t, report.Healthy)
3636
assert.True(t, report.Reachable)
3737
assert.Equal(t, ping, report.Latency)
38-
assert.NoError(t, report.Error)
38+
assert.Nil(t, report.Error)
3939
})
4040

4141
t.Run("Error", func(t *testing.T) {
@@ -56,7 +56,7 @@ func TestDatabase(t *testing.T) {
5656
assert.False(t, report.Healthy)
5757
assert.False(t, report.Reachable)
5858
assert.Zero(t, report.Latency)
59-
assert.ErrorIs(t, report.Error, err)
59+
assert.Nil(t, report.Error, err)
6060
})
6161

6262
t.Run("Median", func(t *testing.T) {
@@ -80,6 +80,6 @@ func TestDatabase(t *testing.T) {
8080
assert.True(t, report.Healthy)
8181
assert.True(t, report.Reachable)
8282
assert.Equal(t, time.Millisecond, report.Latency)
83-
assert.NoError(t, report.Error)
83+
assert.Nil(t, report.Error)
8484
})
8585
}

‎coderd/healthcheck/derp.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ type DERPReport struct {
3030
Regions map[int]*DERPRegionReport `json:"regions"`
3131

3232
Netcheck *netcheck.Report `json:"netcheck"`
33-
NetcheckErr error `json:"netcheck_err"`
33+
NetcheckErr *string `json:"netcheck_err"`
3434
NetcheckLogs []string `json:"netcheck_logs"`
3535

36-
Error error `json:"error"`
36+
Error *string `json:"error"`
3737
}
3838

3939
type DERPRegionReport struct {
@@ -42,7 +42,7 @@ type DERPRegionReport struct {
4242

4343
Region *tailcfg.DERPRegion `json:"region"`
4444
NodeReports []*DERPNodeReport `json:"node_reports"`
45-
Error error `json:"error"`
45+
Error *string `json:"error"`
4646
}
4747
type DERPNodeReport struct {
4848
mu sync.Mutex
@@ -56,8 +56,8 @@ type DERPNodeReport struct {
5656
RoundTripPing time.Duration `json:"round_trip_ping"`
5757
UsesWebsocket bool `json:"uses_websocket"`
5858
ClientLogs [][]string `json:"client_logs"`
59-
ClientErrs [][]error `json:"client_errs"`
60-
Error error `json:"error"`
59+
ClientErrs [][]string `json:"client_errs"`
60+
Error *string `json:"error"`
6161

6262
STUN DERPStunReport `json:"stun"`
6363
}
@@ -91,7 +91,7 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) {
9191
defer wg.Done()
9292
defer func() {
9393
if err := recover(); err != nil {
94-
regionReport.Error = xerrors.Errorf("%v", err)
94+
regionReport.Error = ptr.Ref(fmt.Sprint(err))
9595
}
9696
}()
9797

@@ -115,7 +115,9 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) {
115115
PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil),
116116
Logf: tslogger.WithPrefix(ncLogf, "netcheck: "),
117117
}
118-
r.Netcheck, r.NetcheckErr = nc.GetReport(ctx, opts.DERPMap)
118+
ncReport, netcheckErr := nc.GetReport(ctx, opts.DERPMap)
119+
r.Netcheck = ncReport
120+
r.NetcheckErr = convertError(netcheckErr)
119121

120122
wg.Wait()
121123
}
@@ -140,7 +142,7 @@ func (r *DERPRegionReport) Run(ctx context.Context) {
140142
defer wg.Done()
141143
defer func() {
142144
if err := recover(); err != nil {
143-
nodeReport.Error = xerrors.Errorf("%v", err)
145+
nodeReport.Error = ptr.Ref(fmt.Sprint(err))
144146
}
145147
}()
146148

@@ -179,7 +181,7 @@ func (r *DERPNodeReport) Run(ctx context.Context) {
179181
defer cancel()
180182

181183
r.ClientLogs = [][]string{}
182-
r.ClientErrs = [][]error{}
184+
r.ClientErrs = [][]string{}
183185

184186
wg := &sync.WaitGroup{}
185187

@@ -376,7 +378,7 @@ func (r *DERPNodeReport) stunAddr(ctx context.Context) (string, int, error) {
376378

377379
func (r *DERPNodeReport) writeClientErr(clientID int, err error) {
378380
r.mu.Lock()
379-
r.ClientErrs[clientID] = append(r.ClientErrs[clientID], err)
381+
r.ClientErrs[clientID] = append(r.ClientErrs[clientID], err.Error())
380382
r.mu.Unlock()
381383
}
382384

@@ -385,7 +387,7 @@ func (r *DERPNodeReport) derpClient(ctx context.Context, derpURL *url.URL) (*der
385387
id := r.clientCounter
386388
r.clientCounter++
387389
r.ClientLogs = append(r.ClientLogs, []string{})
388-
r.ClientErrs = append(r.ClientErrs, []error{})
390+
r.ClientErrs = append(r.ClientErrs, []string{})
389391
r.mu.Unlock()
390392

391393
client, err := derphttp.NewClient(key.NewNode(), derpURL.String(), func(format string, args ...any) {

‎coderd/healthcheck/healthcheck.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ package healthcheck
22

33
import (
44
"context"
5+
"fmt"
56
"net/http"
67
"net/url"
78
"sync"
89
"time"
910

10-
"golang.org/x/xerrors"
1111
"tailscale.com/tailcfg"
1212

1313
"github.com/coder/coder/coderd/database"
14+
"github.com/coder/coder/coderd/util/ptr"
1415
)
1516

1617
const (
@@ -51,6 +52,14 @@ type ReportOptions struct {
5152
Checker Checker
5253
}
5354

55+
func convertError(err error) *string {
56+
if err != nil {
57+
return ptr.Ref(err.Error())
58+
}
59+
60+
return nil
61+
}
62+
5463
type defaultChecker struct{}
5564

5665
func (defaultChecker) DERP(ctx context.Context, opts *DERPReportOptions) (report DERPReport) {
@@ -88,7 +97,7 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
8897
defer wg.Done()
8998
defer func() {
9099
if err := recover(); err != nil {
91-
report.DERP.Error = xerrors.Errorf("%v", err)
100+
report.DERP.Error = ptr.Ref(fmt.Sprint(err))
92101
}
93102
}()
94103

@@ -102,7 +111,7 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
102111
defer wg.Done()
103112
defer func() {
104113
if err := recover(); err != nil {
105-
report.AccessURL.Error = xerrors.Errorf("%v", err)
114+
report.AccessURL.Error = ptr.Ref(fmt.Sprint(err))
106115
}
107116
}()
108117

@@ -117,7 +126,7 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
117126
defer wg.Done()
118127
defer func() {
119128
if err := recover(); err != nil {
120-
report.Websocket.Error = xerrors.Errorf("%v", err)
129+
report.Websocket.Error = ptr.Ref(fmt.Sprint(err))
121130
}
122131
}()
123132

@@ -132,7 +141,7 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
132141
defer wg.Done()
133142
defer func() {
134143
if err := recover(); err != nil {
135-
report.Database.Error = xerrors.Errorf("%v", err)
144+
report.Database.Error = ptr.Ref(fmt.Sprint(err))
136145
}
137146
}()
138147

‎coderd/healthcheck/websocket.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ type WebsocketReportOptions struct {
2323
type WebsocketReport struct {
2424
Healthy bool `json:"healthy"`
2525
Response WebsocketResponse `json:"response"`
26-
Error error `json:"error"`
26+
Error *string `json:"error"`
2727
}
2828

2929
type WebsocketResponse struct {
@@ -37,7 +37,7 @@ func (r *WebsocketReport) Run(ctx context.Context, opts *WebsocketReportOptions)
3737

3838
u, err := opts.AccessURL.Parse("/api/v2/debug/ws")
3939
if err != nil {
40-
r.Error = xerrors.Errorf("parse access url: %w", err)
40+
r.Error = convertError(xerrors.Errorf("parse access url: %w", err))
4141
return
4242
}
4343
if u.Scheme == "https" {
@@ -66,7 +66,7 @@ func (r *WebsocketReport) Run(ctx context.Context, opts *WebsocketReportOptions)
6666
}
6767
}
6868
if err != nil {
69-
r.Error = xerrors.Errorf("websocket dial: %w", err)
69+
r.Error = convertError(xerrors.Errorf("websocket dial: %w", err))
7070
return
7171
}
7272
defer c.Close(websocket.StatusGoingAway, "goodbye")
@@ -75,23 +75,23 @@ func (r *WebsocketReport) Run(ctx context.Context, opts *WebsocketReportOptions)
7575
msg := strconv.Itoa(i)
7676
err := c.Write(ctx, websocket.MessageText, []byte(msg))
7777
if err != nil {
78-
r.Error = xerrors.Errorf("write message: %w", err)
78+
r.Error = convertError(xerrors.Errorf("write message: %w", err))
7979
return
8080
}
8181

8282
ty, got, err := c.Read(ctx)
8383
if err != nil {
84-
r.Error = xerrors.Errorf("read message: %w", err)
84+
r.Error = convertError(xerrors.Errorf("read message: %w", err))
8585
return
8686
}
8787

8888
if ty != websocket.MessageText {
89-
r.Error = xerrors.Errorf("received incorrect message type: %v", ty)
89+
r.Error = convertError(xerrors.Errorf("received incorrect message type: %v", ty))
9090
return
9191
}
9292

9393
if string(got) != msg {
94-
r.Error = xerrors.Errorf("received incorrect message: wanted %q, got %q", msg, string(got))
94+
r.Error = convertError(xerrors.Errorf("received incorrect message: wanted %q, got %q", msg, string(got)))
9595
return
9696
}
9797
}

‎coderd/healthcheck/websocket_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestWebsocket(t *testing.T) {
3737
APIKey: "test",
3838
})
3939

40-
require.NoError(t, wsReport.Error)
40+
require.Nil(t, wsReport.Error)
4141
})
4242

4343
t.Run("Error", func(t *testing.T) {
@@ -62,7 +62,7 @@ func TestWebsocket(t *testing.T) {
6262
APIKey: "test",
6363
})
6464

65-
require.Error(t, wsReport.Error)
65+
require.NotNil(t, wsReport.Error)
6666
assert.Equal(t, wsReport.Response.Body, "test error")
6767
assert.Equal(t, wsReport.Response.Code, http.StatusBadRequest)
6868
})

0 commit comments

Comments
 (0)