Skip to content

Commit c7bddf1

Browse files
ktrysmtclaude
ktrysmt
and
claude
authored
fix: replace log.Fatal with proper error returns in client constructors (#327)
* fix: replace log.Fatal with proper error returns in client constructors Replace all log.Fatal calls in client constructor functions with proper error returns to allow library users to handle errors gracefully instead of crashing the entire application. Changes: - Updated NewOAuthClientCredentials to return (*Client, error) - Updated NewOAuth to return (*Client, error) - Updated NewOAuthWithCode to return (*Client, string, error) - Updated NewOAuthWithRefreshToken to return (*Client, string, error) - Updated NewOAuthbearerToken to return (*Client, error) - Updated NewBasicAuth to return (*Client, error) - Updated injectClient to return (*Client, error) - Removed unused log import - Updated all test files to handle the new error returns This ensures the library behaves as a proper library and doesn't force the calling application to exit on authentication errors. Fixes #326 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: add error context wrapping in OAuth client constructors Improve error diagnostics by wrapping errors returned from injectClient with additional context in NewOAuthWithCode and NewOAuthWithRefreshToken. This creates a clearer error chain for callers to inspect and debug. Changes: - Wrap injectClient error in NewOAuthWithCode with "failed to create client" - Wrap injectClient error in NewOAuthWithRefreshToken with "failed to create client" This addresses review feedback from @gemini-code-assist. * deprecate: mark NewOAuth as deprecated due to stdin/stdout usage The NewOAuth function uses stdin/stdout directly, making it unsuitable for non-interactive environments such as web servers and background jobs. This marks the function as deprecated and recommends using NewOAuthWithCode instead, where users can obtain the authorization code through their own UI/CLI implementation. This addresses the design concern raised in review feedback from @gemini-code-assist. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 978aad8 commit c7bddf1

16 files changed

+189
-67
lines changed

‎client.go‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"io"
88
"io/ioutil"
9-
"log"
109
"mime/multipart"
1110
"net/http"
1211
"net/url"
@@ -63,16 +62,16 @@ type auth struct {
6362

6463
type Response struct {
6564
*http.Response `json:"-"`
66-
Size int `json:"size"`
67-
Page int `json:"page"`
68-
Pagelen int `json:"pagelen"`
69-
Next string `json:"next"`
70-
Previous string `json:"previous"`
71-
Values []interface{} `json:"values"`
65+
Size int `json:"size"`
66+
Page int `json:"page"`
67+
Pagelen int `json:"pagelen"`
68+
Next string `json:"next"`
69+
Previous string `json:"previous"`
70+
Values []interface{} `json:"values"`
7271
}
7372

7473
// Uses the Client Credentials Grant oauth2 flow to authenticate to Bitbucket
75-
func NewOAuthClientCredentials(i, s string) *Client {
74+
func NewOAuthClientCredentials(i, s string) (*Client, error) {
7675
a := &auth{appID: i, secret: s}
7776
ctx := context.Background()
7877
conf := &clientcredentials.Config{
@@ -83,14 +82,20 @@ func NewOAuthClientCredentials(i, s string) *Client {
8382

8483
tok, err := conf.Token(ctx)
8584
if err != nil {
86-
log.Fatal(err)
85+
return nil, fmt.Errorf("failed to obtain token: %w", err)
8786
}
8887
a.token = *tok
8988
return injectClient(a)
9089

9190
}
9291

93-
func NewOAuth(i, s string) *Client {
92+
// NewOAuth performs an interactive OAuth flow using stdin/stdout.
93+
//
94+
// Deprecated: This function uses stdin/stdout directly, making it unsuitable for
95+
// non-interactive environments (e.g., web servers, background jobs). Instead, use
96+
// NewOAuthWithCode after obtaining the authorization code through your own UI/CLI.
97+
// You can generate the authorization URL using oauth2.Config.AuthCodeURL() directly.
98+
func NewOAuth(i, s string) (*Client, error) {
9499
a := &auth{appID: i, secret: s}
95100
ctx := context.Background()
96101
conf := &oauth2.Config{
@@ -111,19 +116,19 @@ func NewOAuth(i, s string) *Client {
111116
var code string
112117
fmt.Printf("Enter the code in the return URL: ")
113118
if _, err := fmt.Scan(&code); err != nil {
114-
log.Fatal(err)
119+
return nil, fmt.Errorf("failed to read authorization code: %w", err)
115120
}
116121
tok, err := conf.Exchange(ctx, code)
117122
if err != nil {
118-
log.Fatal(err)
123+
return nil, fmt.Errorf("failed to exchange authorization code: %w", err)
119124
}
120125
a.token = *tok
121126
return injectClient(a)
122127
}
123128

124129
// NewOAuthWithCode finishes the OAuth handshake with a given code
125130
// and returns a *Client
126-
func NewOAuthWithCode(i, s, c string) (*Client, string) {
131+
func NewOAuthWithCode(i, s, c string) (*Client, string, error) {
127132
a := &auth{appID: i, secret: s}
128133
ctx := context.Background()
129134
conf := &oauth2.Config{
@@ -134,15 +139,19 @@ func NewOAuthWithCode(i, s, c string) (*Client, string) {
134139

135140
tok, err := conf.Exchange(ctx, c)
136141
if err != nil {
137-
log.Fatal(err)
142+
return nil, "", fmt.Errorf("failed to exchange authorization code: %w", err)
138143
}
139144
a.token = *tok
140-
return injectClient(a), tok.AccessToken
145+
client, err := injectClient(a)
146+
if err != nil {
147+
return nil, "", fmt.Errorf("failed to create client: %w", err)
148+
}
149+
return client, tok.AccessToken, nil
141150
}
142151

143152
// NewOAuthWithRefreshToken obtains a new access token with a given refresh token
144153
// and returns a *Client
145-
func NewOAuthWithRefreshToken(i, s, rt string) (*Client, string) {
154+
func NewOAuthWithRefreshToken(i, s, rt string) (*Client, string, error) {
146155
a := &auth{appID: i, secret: s}
147156
ctx := context.Background()
148157
conf := &oauth2.Config{
@@ -156,26 +165,30 @@ func NewOAuthWithRefreshToken(i, s, rt string) (*Client, string) {
156165
})
157166
tok, err := tokenSource.Token()
158167
if err != nil {
159-
log.Fatal(err)
168+
return nil, "", fmt.Errorf("failed to refresh token: %w", err)
160169
}
161170
a.token = *tok
162-
return injectClient(a), tok.AccessToken
171+
client, err := injectClient(a)
172+
if err != nil {
173+
return nil, "", fmt.Errorf("failed to create client: %w", err)
174+
}
175+
return client, tok.AccessToken, nil
163176
}
164177

165-
func NewOAuthbearerToken(t string) *Client {
178+
func NewOAuthbearerToken(t string) (*Client, error) {
166179
a := &auth{bearerToken: t}
167180
return injectClient(a)
168181
}
169182

170-
func NewBasicAuth(u, p string) *Client {
183+
func NewBasicAuth(u, p string) (*Client, error) {
171184
a := &auth{user: u, password: p}
172185
return injectClient(a)
173186
}
174187

175-
func injectClient(a *auth) *Client {
188+
func injectClient(a *auth) (*Client, error) {
176189
bitbucketUrl, err := apiBaseUrl()
177190
if err != nil {
178-
log.Fatalf("invalid bitbucket url")
191+
return nil, fmt.Errorf("invalid bitbucket url: %w", err)
179192
}
180193
c := &Client{Auth: a, Pagelen: DEFAULT_PAGE_LENGTH, MaxDepth: DEFAULT_MAX_DEPTH,
181194
apiBaseURL: bitbucketUrl, LimitPages: DEFAULT_LIMIT_PAGES}
@@ -193,14 +206,14 @@ func injectClient(a *auth) *Client {
193206
DeployKeys: &DeployKeys{c: c},
194207
}
195208
c.Users = &Users{
196-
c: c,
209+
c: c,
197210
SSHKeys: &SSHKeys{c: c},
198211
}
199212
c.User = &User{c: c}
200213
c.Teams = &Teams{c: c}
201214
c.Workspaces = &Workspace{c: c, Repositories: c.Repositories, Permissions: &Permission{c: c}}
202215
c.HttpClient = new(http.Client)
203-
return c
216+
return c, nil
204217
}
205218

206219
func (c *Client) GetOAuthToken() oauth2.Token {

‎teams.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ func (t *Teams) Repositories(teamname string) (interface{}, error) {
3535
}
3636

3737
func (t *Teams) Projects(teamname string) (interface{}, error) {
38-
urlStr := t.c.requestUrl("/teams/%s/projects/", teamname)
39-
return t.c.execute("GET", urlStr, "")
38+
urlStr := t.c.requestUrl("/teams/%s/projects/", teamname)
39+
return t.c.execute("GET", urlStr, "")
4040
}
41-

‎tests/branchrestrictions_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ func setup(t *testing.T) *bitbucket.Client {
3232
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
3333
}
3434

35-
c := bitbucket.NewBasicAuth(user, pass)
35+
c, err := bitbucket.NewBasicAuth(user, pass)
36+
if err != nil {
37+
t.Fatal(err)
38+
}
3639
return c
3740
}
3841

‎tests/client_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88

99
func TestClientNewBasicAuth(t *testing.T) {
1010

11-
c := bitbucket.NewBasicAuth("example", "password")
11+
c, err := bitbucket.NewBasicAuth("example", "password")
12+
if err != nil {
13+
t.Fatal(err)
14+
}
1215

1316
r := reflect.ValueOf(c)
1417

‎tests/deploykeys_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ func TestDeployKey(t *testing.T) {
2929
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
3030
}
3131

32-
c := bitbucket.NewBasicAuth(user, pass)
32+
c, err := bitbucket.NewBasicAuth(user, pass)
33+
if err != nil {
34+
t.Fatal(err)
35+
}
3336

3437
var deployKeyResourceId int
3538

@@ -121,7 +124,10 @@ func TestDeployKeyWithComment(t *testing.T) {
121124
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
122125
}
123126

124-
c := bitbucket.NewBasicAuth(user, pass)
127+
c, err := bitbucket.NewBasicAuth(user, pass)
128+
if err != nil {
129+
t.Fatal(err)
130+
}
125131

126132
var deployKeyResourceId int
127133

@@ -220,7 +226,10 @@ func TestListDeployKeys(t *testing.T) {
220226
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
221227
}
222228

223-
c := bitbucket.NewBasicAuth(user, pass)
229+
c, err := bitbucket.NewBasicAuth(user, pass)
230+
if err != nil {
231+
t.Fatal(err)
232+
}
224233

225234
var deployKeyResourceId1 int
226235
var deployKeyResourceId2 int

‎tests/diff_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ func TestDiff(t *testing.T) {
2323
t.Error("BITBUCKET_TEST_PASSWORD is empty.")
2424
}
2525

26-
c := bitbucket.NewBasicAuth(user, pass)
26+
c, err := bitbucket.NewBasicAuth(user, pass)
27+
if err != nil {
28+
t.Fatal(err)
29+
}
2730

2831
spec := "master..develop"
2932

@@ -59,7 +62,10 @@ func TestGetDiffStat(t *testing.T) {
5962
t.Error("BITBUCKET_TEST_PASSWORD is empty.")
6063
}
6164

62-
c := bitbucket.NewBasicAuth(user, pass)
65+
c, err := bitbucket.NewBasicAuth(user, pass)
66+
if err != nil {
67+
t.Fatal(err)
68+
}
6369

6470
spec := "master..develop"
6571

@@ -95,7 +101,10 @@ func TestGetDiffStatWithFields(t *testing.T) {
95101
t.Error("BITBUCKET_TEST_PASSWORD is empty.")
96102
}
97103

98-
c := bitbucket.NewBasicAuth(user, pass)
104+
c, err := bitbucket.NewBasicAuth(user, pass)
105+
if err != nil {
106+
t.Fatal(err)
107+
}
99108

100109
spec := "master..develop"
101110

‎tests/environment_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ func TestListEnvironments(t *testing.T) {
2828
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
2929
}
3030

31-
c := bitbucket.NewBasicAuth(user, pass)
31+
c, err := bitbucket.NewBasicAuth(user, pass)
32+
if err != nil {
33+
t.Fatal(err)
34+
}
3235

3336
opt := &bitbucket.RepositoryEnvironmentsOptions{
3437
Owner: owner,
@@ -65,7 +68,10 @@ func TestEndToEndEnvironments(t *testing.T) {
6568
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
6669
}
6770

68-
c := bitbucket.NewBasicAuth(user, pass)
71+
c, err := bitbucket.NewBasicAuth(user, pass)
72+
if err != nil {
73+
t.Fatal(err)
74+
}
6975

7076
opt := &bitbucket.RepositoryEnvironmentOptions{
7177
Owner: owner,

‎tests/list_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ func TestList(t *testing.T) {
1414
pass := os.Getenv("BITBUCKET_TEST_PASSWORD")
1515
owner := os.Getenv("BITBUCKET_TEST_OWNER")
1616

17-
c := bitbucket.NewBasicAuth(user, pass)
17+
c, err := bitbucket.NewBasicAuth(user, pass)
18+
if err != nil {
19+
t.Fatal(err)
20+
}
1821

1922
opt := &bitbucket.RepositoriesOptions{
2023
Owner: owner,

‎tests/project_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ func getClient(t *testing.T) *bitbucket.Client {
1818
t.Error("BITBUCKET_TEST_PASSWORD is empty.")
1919
}
2020

21-
return bitbucket.NewBasicAuth(user, pass)
21+
c, err := bitbucket.NewBasicAuth(user, pass)
22+
if err != nil {
23+
t.Fatal(err)
24+
}
25+
return c
2226
}
2327

2428
func getOwner(t *testing.T) string {

‎tests/repositories_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ func TestListForAccount(t *testing.T) {
2626
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
2727
}
2828

29-
c := bitbucket.NewBasicAuth(user, pass)
29+
c, err := bitbucket.NewBasicAuth(user, pass)
30+
if err != nil {
31+
t.Fatal(err)
32+
}
3033

3134
repositories, err := c.Repositories.ListForAccount(&bitbucket.RepositoriesOptions{
3235
Owner: owner,
@@ -66,7 +69,10 @@ func TestListForAccountWithKeyword(t *testing.T) {
6669
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
6770
}
6871

69-
c := bitbucket.NewBasicAuth(user, pass)
72+
c, err := bitbucket.NewBasicAuth(user, pass)
73+
if err != nil {
74+
t.Fatal(err)
75+
}
7076
t.Run("only keyword", func(t *testing.T) {
7177
repositories, err := c.Repositories.ListForAccount(&bitbucket.RepositoriesOptions{
7278
Owner: owner,
@@ -129,7 +135,10 @@ func TestListForTeam(t *testing.T) {
129135
t.Error("BITBUCKET_TEST_REPOSLUG is empty.")
130136
}
131137

132-
c := bitbucket.NewBasicAuth(user, pass)
138+
c, err := bitbucket.NewBasicAuth(user, pass)
139+
if err != nil {
140+
t.Fatal(err)
141+
}
133142

134143
//goland:noinspection GoDeprecation
135144
repositories, err := c.Repositories.ListForTeam(&bitbucket.RepositoriesOptions{

0 commit comments

Comments
 (0)