Skip to content

Commit

Permalink
fix: got error response when create token (#152)
Browse files Browse the repository at this point in the history
The empty redirect_url shown in request will cause error when user
authorization url without redirect_url(it's valid case).

Also fix parse create token error response to empty error object.
  • Loading branch information
ronazst authored Aug 8, 2023
1 parent 038f987 commit a86cf10
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 16 deletions.
13 changes: 11 additions & 2 deletions authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type AuthenticationClient struct {
//
// See https://developers.notion.com/reference/create-a-token
func (cc *AuthenticationClient) CreateToken(ctx context.Context, request *TokenCreateRequest) (*TokenCreateResponse, error) {
res, err := cc.apiClient.requestImpl(ctx, http.MethodPost, "oauth/token", nil, request, true)
res, err := cc.apiClient.requestImpl(ctx, http.MethodPost, "oauth/token", nil, request, true, decodeTokenCreateError)
if err != nil {
return nil, err
}
Expand All @@ -40,6 +40,15 @@ func (cc *AuthenticationClient) CreateToken(ctx context.Context, request *TokenC
return &response, nil
}

func decodeTokenCreateError(data []byte) error {
var apiErr TokenCreateError
err := json.Unmarshal(data, &apiErr)
if err != nil {
return err
}
return &apiErr
}

// TokenCreateRequest represents the request body for AuthenticationClient.CreateToken.
type TokenCreateRequest struct {
// A unique random code that Notion generates to authenticate with your service,
Expand All @@ -51,7 +60,7 @@ type TokenCreateRequest struct {
// the integration's Authorization settings. Do not include this field if a
// "redirect_uri" query param was not included in the Authorization URL
// provided to users. In most cases, this field is required.
RedirectUri string `json:"redirect_uri"`
RedirectUri string `json:"redirect_uri,omitempty"`
// Required if and only when building Link Preview integrations (otherwise
// ignored). An object with key and name properties. key should be a unique
// identifier for the account. Notion uses the key to determine whether or not
Expand Down
27 changes: 19 additions & 8 deletions authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ func TestAuthenticationClient(t *testing.T) {
statusCode int
request *notionapi.TokenCreateRequest
want *notionapi.TokenCreateResponse
wantErr bool
err error
wantErr error
}{
{
name: "Creates token",
Expand All @@ -37,20 +36,32 @@ func TestAuthenticationClient(t *testing.T) {
WorkspaceId: "workspaceid_1",
WorkspaceName: "workspace_1",
},
wantErr: false,
err: nil,
wantErr: nil,
},
{
name: "Creates token",
filePath: "testdata/create_token_error.json",
statusCode: http.StatusBadRequest,
request: &notionapi.TokenCreateRequest{
Code: "code1",
GrantType: "authorization_code",
RedirectUri: "www.example.com",
},
wantErr: &notionapi.TokenCreateError{
Code: "invalid_grant",
Message: "Invalid code.",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := newMockedClient(t, tt.filePath, tt.statusCode)
client := notionapi.NewClient("some_token", notionapi.WithHTTPClient(c))
got, err := client.Authentication.CreateToken(context.Background(), tt.request)
got, gotErr := client.Authentication.CreateToken(context.Background(), tt.request)

if (err != nil) != tt.wantErr {
t.Errorf("Query() error = %v, wantErr %v", err, tt.wantErr)
return
if !reflect.DeepEqual(gotErr, tt.wantErr) {
t.Errorf("Query() gotErr = %v, wantErr %v", gotErr, tt.wantErr)
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Query() got = %v, want %v", got, tt.want)
Expand Down
22 changes: 16 additions & 6 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/url"
"reflect"
Expand All @@ -23,6 +24,8 @@ const (

type Token string

type errJsonDecodeFunc func(data []byte) error

func (it Token) String() string {
return string(it)
}
Expand Down Expand Up @@ -112,10 +115,10 @@ func WithOAuthAppCredentials(id, secret string) ClientOption {
}

func (c *Client) request(ctx context.Context, method string, urlStr string, queryParams map[string]string, requestBody interface{}) (*http.Response, error) {
return c.requestImpl(ctx, method, urlStr, queryParams, requestBody, false)
return c.requestImpl(ctx, method, urlStr, queryParams, requestBody, false, decodeClientError)
}

func (c *Client) requestImpl(ctx context.Context, method string, urlStr string, queryParams map[string]string, requestBody interface{}, basicAuth bool) (*http.Response, error) {
func (c *Client) requestImpl(ctx context.Context, method string, urlStr string, queryParams map[string]string, requestBody interface{}, basicAuth bool, errDecoder errJsonDecodeFunc) (*http.Response, error) {
u, err := c.baseUrl.Parse(fmt.Sprintf("%s/%s", c.apiVersion, urlStr))
if err != nil {
return nil, err
Expand Down Expand Up @@ -187,18 +190,25 @@ func (c *Client) requestImpl(ctx context.Context, method string, urlStr string,
}

if res.StatusCode != http.StatusOK {
var apiErr Error
err = json.NewDecoder(res.Body).Decode(&apiErr)
data, err := ioutil.ReadAll(res.Body)
if err != nil {
return nil, err
}

return nil, &apiErr
return nil, errDecoder(data)
}

return res, nil
}

func decodeClientError(data []byte) error {
var apiErr Error
err := json.Unmarshal(data, &apiErr)
if err != nil {
return err
}
return &apiErr
}

type Pagination struct {
StartCursor Cursor
PageSize int
Expand Down
9 changes: 9 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,12 @@ type RateLimitedError struct {
func (e *RateLimitedError) Error() string {
return e.Message
}

type TokenCreateError struct {
Code ErrorCode `json:"error"`
Message string `json:"error_description"`
}

func (e *TokenCreateError) Error() string {
return e.Message
}
4 changes: 4 additions & 0 deletions testdata/create_token_error.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"error": "invalid_grant",
"error_description": "Invalid code."
}

0 comments on commit a86cf10

Please sign in to comment.