Skip to content

Commit e332acd

Browse files
appleboyclaude
andcommitted
refactor: fix double error wrapping and harden code quality
- Eliminate double error wrapping on readResponseBody call sites - Replace unsafe type assertion on SecureStore with local variable - Add constants for token store backend modes (auto, file, keyring) - Use specific error messages in exchangeDeviceCode and verifyToken - Remove obvious WHAT comments that duplicate code intent - Remove unnecessary generic type argument on NewSecureStore Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 61d8569 commit e332acd

1 file changed

Lines changed: 22 additions & 25 deletions

File tree

main.go

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ const (
9191
oauthErrInvalidToken = "invalid_token"
9292
)
9393

94+
// Token store backend modes for the -token-store flag
95+
const (
96+
tokenStoreModeAuto = "auto"
97+
tokenStoreModeFile = "file"
98+
tokenStoreModeKeyring = "keyring"
99+
)
100+
94101
// tokenResponse is the common structure for OAuth token endpoint responses.
95102
type tokenResponse struct {
96103
AccessToken string `json:"access_token"`
@@ -208,14 +215,15 @@ func doInitConfig() {
208215
// Initialize token store based on mode
209216
fileStore := credstore.NewTokenFileStore(tokenFile)
210217
switch tokenStoreMode {
211-
case "file":
218+
case tokenStoreModeFile:
212219
tokenStore = fileStore
213-
case "keyring":
220+
case tokenStoreModeKeyring:
214221
tokenStore = credstore.NewTokenKeyringStore(defaultKeyringService)
215-
case "auto":
222+
case tokenStoreModeAuto:
216223
kr := credstore.NewTokenKeyringStore(defaultKeyringService)
217-
tokenStore = credstore.NewSecureStore[credstore.Token](kr, fileStore)
218-
if !tokenStore.(*credstore.SecureStore[credstore.Token]).UseKeyring() {
224+
secureStore := credstore.NewSecureStore(kr, fileStore)
225+
tokenStore = secureStore
226+
if !secureStore.UseKeyring() {
219227
fmt.Fprintln(
220228
os.Stderr,
221229
"⚠️ OS keyring unavailable, falling back to file-based token storage",
@@ -224,8 +232,8 @@ func doInitConfig() {
224232
default:
225233
fmt.Fprintf(
226234
os.Stderr,
227-
"Error: Invalid token-store value: %s (must be auto, file, or keyring)\n",
228-
tokenStoreMode,
235+
"Error: Invalid token-store value: %s (must be %s, %s, or %s)\n",
236+
tokenStoreMode, tokenStoreModeAuto, tokenStoreModeFile, tokenStoreModeKeyring,
229237
)
230238
os.Exit(1)
231239
}
@@ -432,7 +440,6 @@ func run(ctx context.Context, d tui.Displayer) error {
432440

433441
// requestDeviceCode requests a device code from the OAuth server with retry logic
434442
func requestDeviceCode(ctx context.Context) (*oauth2.DeviceAuthResponse, error) {
435-
// Create request with timeout
436443
reqCtx, cancel := context.WithTimeout(ctx, deviceCodeRequestTimeout)
437444
defer cancel()
438445

@@ -451,7 +458,6 @@ func requestDeviceCode(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
451458
}
452459
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
453460

454-
// Execute request with retry logic
455461
resp, err := retryClient.DoWithContext(reqCtx, req)
456462
if err != nil {
457463
return nil, fmt.Errorf("device code request failed: %w", err)
@@ -460,7 +466,7 @@ func requestDeviceCode(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
460466

461467
body, err := readResponseBody(resp.Body)
462468
if err != nil {
463-
return nil, fmt.Errorf("failed to read response: %w", err)
469+
return nil, err
464470
}
465471

466472
if resp.StatusCode != http.StatusOK {
@@ -471,7 +477,6 @@ func requestDeviceCode(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
471477
)
472478
}
473479

474-
// Parse response
475480
var deviceResp struct {
476481
DeviceCode string `json:"device_code"`
477482
UserCode string `json:"user_code"`
@@ -627,7 +632,6 @@ func exchangeDeviceCode(
627632
ctx context.Context,
628633
tokenURL, clientID, deviceCode string,
629634
) (*oauth2.Token, error) {
630-
// Create request with timeout
631635
reqCtx, cancel := context.WithTimeout(ctx, tokenExchangeTimeout)
632636
defer cancel()
633637

@@ -649,24 +653,22 @@ func exchangeDeviceCode(
649653

650654
resp, err := retryClient.DoWithContext(reqCtx, req)
651655
if err != nil {
652-
return nil, fmt.Errorf("request failed: %w", err)
656+
return nil, fmt.Errorf("token exchange request failed: %w", err)
653657
}
654658
defer resp.Body.Close()
655659

656660
body, err := readResponseBody(resp.Body)
657661
if err != nil {
658-
return nil, fmt.Errorf("failed to read response: %w", err)
662+
return nil, err
659663
}
660664

661-
// Handle non-200 responses
662665
if resp.StatusCode != http.StatusOK {
663666
return nil, &oauth2.RetrieveError{
664667
Response: resp,
665668
Body: body,
666669
}
667670
}
668671

669-
// Parse successful token response
670672
var tokenResp tokenResponse
671673
if err := json.Unmarshal(body, &tokenResp); err != nil {
672674
return nil, fmt.Errorf("failed to parse token response: %w", err)
@@ -692,7 +694,6 @@ func exchangeDeviceCode(
692694
}
693695

694696
func verifyToken(ctx context.Context, accessToken string, d tui.Displayer) error {
695-
// Create request with timeout
696697
reqCtx, cancel := context.WithTimeout(ctx, tokenVerificationTimeout)
697698
defer cancel()
698699

@@ -704,16 +705,15 @@ func verifyToken(ctx context.Context, accessToken string, d tui.Displayer) error
704705
}
705706
req.Header.Set("Authorization", "Bearer "+accessToken)
706707

707-
// Execute request with retry logic
708708
resp, err := retryClient.DoWithContext(reqCtx, req)
709709
if err != nil {
710-
return fmt.Errorf("request failed: %w", err)
710+
return fmt.Errorf("token verification request failed: %w", err)
711711
}
712712
defer resp.Body.Close()
713713

714714
body, err := readResponseBody(resp.Body)
715715
if err != nil {
716-
return fmt.Errorf("failed to read response: %w", err)
716+
return err
717717
}
718718

719719
if resp.StatusCode != http.StatusOK {
@@ -734,7 +734,6 @@ func refreshAccessToken(
734734
refreshToken string,
735735
d tui.Displayer,
736736
) (credstore.Token, error) {
737-
// Create request with timeout
738737
reqCtx, cancel := context.WithTimeout(ctx, refreshTokenTimeout)
739738
defer cancel()
740739

@@ -754,7 +753,6 @@ func refreshAccessToken(
754753
}
755754
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
756755

757-
// Execute request with retry logic
758756
resp, err := retryClient.DoWithContext(reqCtx, req)
759757
if err != nil {
760758
return credstore.Token{}, fmt.Errorf("refresh request failed: %w", err)
@@ -763,7 +761,7 @@ func refreshAccessToken(
763761

764762
body, err := readResponseBody(resp.Body)
765763
if err != nil {
766-
return credstore.Token{}, fmt.Errorf("failed to read response: %w", err)
764+
return credstore.Token{}, err
767765
}
768766

769767
if resp.StatusCode != http.StatusOK {
@@ -782,7 +780,6 @@ func refreshAccessToken(
782780
)
783781
}
784782

785-
// Parse token response
786783
var tokenResp tokenResponse
787784
if err := json.Unmarshal(body, &tokenResp); err != nil {
788785
return credstore.Token{}, fmt.Errorf("failed to parse token response: %w", err)
@@ -891,7 +888,7 @@ func makeAPICallWithAutoRefresh(
891888

892889
body, err := readResponseBody(resp.Body)
893890
if err != nil {
894-
return fmt.Errorf("failed to read response: %w", err)
891+
return err
895892
}
896893

897894
if resp.StatusCode != http.StatusOK {

0 commit comments

Comments
 (0)