Skip to content

Commit 693193f

Browse files
mislavmislav
committed
Consistent error handling in codespaces API operations
Using HandleHTTPError ensures that hints regarding insufficient OAuth scopes will be properly reported on stderr.
1 parent 2c3f02e commit 693193f

1 file changed

Lines changed: 35 additions & 68 deletions

File tree

‎internal/codespaces/api/api.go‎

Lines changed: 35 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ func (a *API) GetUser(ctx context.Context) (*User, error) {
8585
}
8686
defer resp.Body.Close()
8787

88+
if resp.StatusCode != http.StatusOK {
89+
return nil, api.HandleHTTPError(resp)
90+
}
91+
8892
b, err := ioutil.ReadAll(resp.Body)
8993
if err != nil {
9094
return nil, fmt.Errorf("error reading response body: %w", err)
9195
}
9296

93-
if resp.StatusCode != http.StatusOK {
94-
return nil, jsonErrorResponse(b)
95-
}
96-
9797
var response User
9898
if err := json.Unmarshal(b, &response); err != nil {
9999
return nil, fmt.Errorf("error unmarshaling response: %w", err)
@@ -102,18 +102,6 @@ func (a *API) GetUser(ctx context.Context) (*User, error) {
102102
return &response, nil
103103
}
104104

105-
// jsonErrorResponse returns the error message from a JSON response.
106-
func jsonErrorResponse(b []byte) error {
107-
var response struct {
108-
Message string `json:"message"`
109-
}
110-
if err := json.Unmarshal(b, &response); err != nil {
111-
return fmt.Errorf("error unmarshaling error response: %w", err)
112-
}
113-
114-
return errors.New(response.Message)
115-
}
116-
117105
// Repository represents a GitHub repository.
118106
type Repository struct {
119107
ID int `json:"id"`
@@ -133,15 +121,15 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error
133121
}
134122
defer resp.Body.Close()
135123

124+
if resp.StatusCode != http.StatusOK {
125+
return nil, api.HandleHTTPError(resp)
126+
}
127+
136128
b, err := ioutil.ReadAll(resp.Body)
137129
if err != nil {
138130
return nil, fmt.Errorf("error reading response body: %w", err)
139131
}
140132

141-
if resp.StatusCode != http.StatusOK {
142-
return nil, jsonErrorResponse(b)
143-
}
144-
145133
var response Repository
146134
if err := json.Unmarshal(b, &response); err != nil {
147135
return nil, fmt.Errorf("error unmarshaling response: %w", err)
@@ -286,15 +274,15 @@ func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeCon
286274
}
287275
defer resp.Body.Close()
288276

277+
if resp.StatusCode != http.StatusOK {
278+
return nil, api.HandleHTTPError(resp)
279+
}
280+
289281
b, err := ioutil.ReadAll(resp.Body)
290282
if err != nil {
291283
return nil, fmt.Errorf("error reading response body: %w", err)
292284
}
293285

294-
if resp.StatusCode != http.StatusOK {
295-
return nil, jsonErrorResponse(b)
296-
}
297-
298286
var response Codespace
299287
if err := json.Unmarshal(b, &response); err != nil {
300288
return nil, fmt.Errorf("error unmarshaling response: %w", err)
@@ -322,26 +310,12 @@ func (a *API) StartCodespace(ctx context.Context, codespaceName string) error {
322310
}
323311
defer resp.Body.Close()
324312

325-
b, err := ioutil.ReadAll(resp.Body)
326-
if err != nil {
327-
return fmt.Errorf("error reading response body: %w", err)
328-
}
329-
330313
if resp.StatusCode != http.StatusOK {
331314
if resp.StatusCode == http.StatusConflict {
332315
// 409 means the codespace is already running which we can safely ignore
333316
return nil
334317
}
335-
336-
// Error response may be a numeric code or a JSON {"message": "..."}.
337-
if bytes.HasPrefix(b, []byte("{")) {
338-
return jsonErrorResponse(b) // probably JSON
339-
}
340-
341-
if len(b) > 100 {
342-
b = append(b[:97], "..."...)
343-
}
344-
return fmt.Errorf("failed to start codespace: %s (%s)", b, resp.Status)
318+
return api.HandleHTTPError(resp)
345319
}
346320

347321
return nil
@@ -364,15 +338,15 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) {
364338
}
365339
defer resp.Body.Close()
366340

341+
if resp.StatusCode != http.StatusOK {
342+
return "", api.HandleHTTPError(resp)
343+
}
344+
367345
b, err := ioutil.ReadAll(resp.Body)
368346
if err != nil {
369347
return "", fmt.Errorf("error reading response body: %w", err)
370348
}
371349

372-
if resp.StatusCode != http.StatusOK {
373-
return "", jsonErrorResponse(b)
374-
}
375-
376350
var response getCodespaceRegionLocationResponse
377351
if err := json.Unmarshal(b, &response); err != nil {
378352
return "", fmt.Errorf("error unmarshaling response: %w", err)
@@ -406,15 +380,15 @@ func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, loc
406380
}
407381
defer resp.Body.Close()
408382

383+
if resp.StatusCode != http.StatusOK {
384+
return nil, api.HandleHTTPError(resp)
385+
}
386+
409387
b, err := ioutil.ReadAll(resp.Body)
410388
if err != nil {
411389
return nil, fmt.Errorf("error reading response body: %w", err)
412390
}
413391

414-
if resp.StatusCode != http.StatusOK {
415-
return nil, jsonErrorResponse(b)
416-
}
417-
418392
var response struct {
419393
Machines []*Machine `json:"machines"`
420394
}
@@ -499,18 +473,17 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca
499473
}
500474
defer resp.Body.Close()
501475

476+
if resp.StatusCode == http.StatusAccepted {
477+
return nil, errProvisioningInProgress // RPC finished before result of creation known
478+
} else if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
479+
return nil, api.HandleHTTPError(resp)
480+
}
481+
502482
b, err := ioutil.ReadAll(resp.Body)
503483
if err != nil {
504484
return nil, fmt.Errorf("error reading response body: %w", err)
505485
}
506486

507-
switch {
508-
case resp.StatusCode > http.StatusAccepted:
509-
return nil, jsonErrorResponse(b)
510-
case resp.StatusCode == http.StatusAccepted:
511-
return nil, errProvisioningInProgress // RPC finished before result of creation known
512-
}
513-
514487
var response Codespace
515488
if err := json.Unmarshal(b, &response); err != nil {
516489
return nil, fmt.Errorf("error unmarshaling response: %w", err)
@@ -533,12 +506,8 @@ func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error {
533506
}
534507
defer resp.Body.Close()
535508

536-
if resp.StatusCode > http.StatusAccepted {
537-
b, err := ioutil.ReadAll(resp.Body)
538-
if err != nil {
539-
return fmt.Errorf("error reading response body: %w", err)
540-
}
541-
return jsonErrorResponse(b)
509+
if resp.StatusCode != http.StatusOK {
510+
return api.HandleHTTPError(resp)
542511
}
543512

544513
return nil
@@ -567,17 +536,15 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod
567536

568537
if resp.StatusCode == http.StatusNotFound {
569538
return nil, nil
539+
} else if resp.StatusCode != http.StatusOK {
540+
return nil, api.HandleHTTPError(resp)
570541
}
571542

572543
b, err := ioutil.ReadAll(resp.Body)
573544
if err != nil {
574545
return nil, fmt.Errorf("error reading response body: %w", err)
575546
}
576547

577-
if resp.StatusCode != http.StatusOK {
578-
return nil, jsonErrorResponse(b)
579-
}
580-
581548
var response getCodespaceRepositoryContentsResponse
582549
if err := json.Unmarshal(b, &response); err != nil {
583550
return nil, fmt.Errorf("error unmarshaling response: %w", err)
@@ -605,14 +572,14 @@ func (a *API) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) {
605572
}
606573
defer resp.Body.Close()
607574

575+
if resp.StatusCode != http.StatusOK {
576+
return nil, fmt.Errorf("server returned %s", resp.Status)
577+
}
578+
608579
b, err := ioutil.ReadAll(resp.Body)
609580
if err != nil {
610581
return nil, fmt.Errorf("error reading response body: %w", err)
611582
}
612-
613-
if resp.StatusCode != http.StatusOK {
614-
return nil, fmt.Errorf("server returned %s", resp.Status)
615-
}
616583
return b, nil
617584
}
618585

0 commit comments

Comments
 (0)