Skip to content

deal with 202 Accepted status code #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
caarlos0 wants to merge 11 commits into from
Closed

deal with 202 Accepted status code #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0c2af3c
deal with 202
caarlos0 Dec 8, 2016
615c613
improved messages, unused pointer, etc
caarlos0 Dec 9, 2016
fbb47d2
Merge branch 'master' into 202
caarlos0 Dec 9, 2016
936aa01
pointer again, docs
caarlos0 Dec 9, 2016
0c97936
repos stats docs
caarlos0 Dec 9, 2016
5887906
repo forks docs
caarlos0 Dec 9, 2016
3468168
same formating
caarlos0 Dec 9, 2016
ba0dc2c
docs
caarlos0 Dec 9, 2016
50886af
Minor typo fixes in README.md and doc.go.
dmitshur Dec 10, 2016
a65a64f
adding spaces back
caarlos0 Dec 10, 2016
63045a5
typo
caarlos0 Dec 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,23 @@ if _, ok := err.(*github.RateLimitError); ok {
Learn more about GitHub rate limiting at
http://developer.github.com/v3/#rate-limiting.

### Accepted Status ###

Some endpoints may return a 202 Accepted status code, meaning that the
information required is not yet ready and was scheduled to be gathered on
GitHub side. Methods known to behave like this are documented specifying
this behavior.

To detect this condition of error, you can check if its type is
`*github.AcceptedError`:

```go
stats, _, err := client.Repositories.ListContributorsStats(org, repo)
if _, ok := err.(*github.AcceptedError); ok {
log.Println("scheduled on github side")
}
```

### Conditional Requests ###

The GitHub API has good support for conditional requests which will help
Expand Down
15 changes: 15 additions & 0 deletions github/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,21 @@ To detect an API rate limit error, you can check if its type is *github.RateLimi
Learn more about GitHub rate limiting at
http://developer.github.com/v3/#rate-limiting.

Accepted Status

Some endpoints may return a 202 Accepted status code, meaning that the
information required is not yet ready and was scheduled to be gathered on
GitHub side. Methods known to behave like this are documented specifying
this behavior.

To detect this condition of error, you can check if its type is
*github.AcceptedError:

stats, _, err := client.Repositories.ListContributorsStats(org, repo)
if _, ok := err.(*github.AcceptedError); ok {
log.Println("scheduled on github side")
}

Conditional Requests

The GitHub API has good support for conditional requests which will help
Expand Down
21 changes: 19 additions & 2 deletions github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,18 @@ func (r *RateLimitError) Error() string {
r.Response.StatusCode, r.Message, r.Rate.Reset.Time.Sub(time.Now()))
}

// AcceptedError occurs when GitHub returns 202 Accepted response with an
// empty body, which means a job was scheduled on the GitHub side to process
// the information needed and cache it.
// Technically, 202 Accepted is not a real error, it's just used to
// indicate that results are not ready yet, but should be available soon.
// The request can be repeated after some time.
type AcceptedError struct{}
Copy link
Copy Markdown
Member

@dmitshur dmitshur Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's obvious or worth adding another sentence to the current documentation, something like:

Technically, 202 Accepted is not a real error, it's just used to indicate that results are not ready yet, but should be available soon. The request should be repeated after some time.

The part that's missing to me is that the documentation should indicate that the this is a "retryable" error. Trying again after some time is completely normal and expected to succeed (but maybe not on the first try).


func (*AcceptedError) Error() string {
return "job scheduled on github side, try again later"
}

// AbuseRateLimitError occurs when GitHub returns 403 Forbidden response with the
// "documentation_url" field value equal to "https://developer.github.com/v3#abuse-rate-limits".
type AbuseRateLimitError struct {
Expand Down Expand Up @@ -564,14 +576,19 @@ func (e *Error) Error() string {
}

// CheckResponse checks the API response for errors, and returns them if
// present. A response is considered an error if it has a status code outside
// the 200 range. API error responses are expected to have either no response
// present. A response is considered an error if it has a status code outside
// the 200 range or equal to 202 Accepted.
// API error responses are expected to have either no response
// body, or a JSON response body that maps to ErrorResponse. Any other
// response body will be silently ignored.
//
// The error type will be *RateLimitError for rate limit exceeded errors,
// *AcceptedError for 202 Accepted status codes,
// and *TwoFactorAuthError for two-factor authentication errors.
Copy link
Copy Markdown
Member

@dmitshur dmitshur Dec 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mention AcceptedError here.

// The error type will be *RateLimitError for rate limit exceeded errors,
// AcceptedError for 202 Accepted status codes,
// and *TwoFactorAuthError for two-factor authentication errors.

This makes me realize that there's actually a good reason to revert back to using a pointer to *AcceptedError rather than value: for consistency of type assertion code to other error types. Currently, it'd be:

switch err.(type) {
case *github.RateLimitError:
	// handle rate limit exceeded error
case *github.TwoFactorAuthError:
	// handle two-factor authentication error
case github.AcceptedError: // inconsistent
	// handle accepted error
}

If we revert to using a pointer to *AcceptedError, it becomes consistent.

switch err.(type) {
case *github.RateLimitError:
	// handle rate limit exceeded error
case *github.TwoFactorAuthError:
	// handle two-factor authentication error
case *github.AcceptedError: // consistent
	// handle accepted error
}

I think that's a good argument to use pointer here too. Do you agree?

Copy link
Copy Markdown
Contributor Author

@caarlos0 caarlos0 Dec 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree... looks more consistent indeed

func CheckResponse(r *http.Response) error {
if r.StatusCode == http.StatusAccepted {
return &AcceptedError{}
}
if c := r.StatusCode; 200 <= c && c <= 299 {
return nil
}
Expand Down
6 changes: 6 additions & 0 deletions github/repos_forks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ type RepositoryCreateForkOptions struct {

// CreateFork creates a fork of the specified repository.
//
// This method might return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing creating the fork in a background task.
// A follow up request, after a delay of a second or so, should result
// in a successful request.
//
// GitHub API docs: https://developer.github.com/v3/repos/forks/#create-a-fork
func (s *RepositoriesService) CreateFork(owner, repo string, opt *RepositoryCreateForkOptions) (*Repository, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/forks", owner, repo)
Expand Down
25 changes: 18 additions & 7 deletions github/repos_stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (w WeeklyStats) String() string {
// deletions and commit counts.
//
// If this is the first time these statistics are requested for the given
// repository, this method will return a non-nil error and a status code of
// repository, this method will return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing the requested statistics. A follow up request, after a
// delay of a second or so, should result in a successful request.
Expand Down Expand Up @@ -78,7 +78,7 @@ func (w WeeklyCommitActivity) String() string {
// starting on Sunday.
//
// If this is the first time these statistics are requested for the given
// repository, this method will return a non-nil error and a status code of
// repository, this method will return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing the requested statistics. A follow up request, after a
// delay of a second or so, should result in a successful request.
Copy link
Copy Markdown
Member

@dmitshur dmitshur Dec 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't see we already had that written up. In that case, it makes a lot of sense to update it as you've done, and copy it over to the other methods (as you've also already done).

Copy link
Copy Markdown
Contributor Author

@caarlos0 caarlos0 Dec 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I haven't seen that before either =)

Expand All @@ -104,6 +104,12 @@ func (s *RepositoriesService) ListCommitActivity(owner, repo string) ([]*WeeklyC
// deletions pushed to a repository. Returned WeeklyStats will contain
// additions and deletions, but not total commits.
//
// If this is the first time these statistics are requested for the given
// repository, this method will return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing the requested statistics. A follow up request, after a
// delay of a second or so, should result in a successful request.
//
// GitHub API Docs: https://developer.github.com/v3/repos/statistics/#code-frequency
func (s *RepositoriesService) ListCodeFrequency(owner, repo string) ([]*WeeklyStats, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/stats/code_frequency", owner, repo)
Expand Down Expand Up @@ -152,11 +158,10 @@ func (r RepositoryParticipation) String() string {
// The array order is oldest week (index 0) to most recent week.
//
// If this is the first time these statistics are requested for the given
// repository, this method will return a non-nil error and a status code
// of 202. This is because this is the status that github returns to
// signify that it is now computing the requested statistics. A follow
// up request, after a delay of a second or so, should result in a
// successful request.
// repository, this method will return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing the requested statistics. A follow up request, after a
// delay of a second or so, should result in a successful request.
//
// GitHub API Docs: https://developer.github.com/v3/repos/statistics/#participation
func (s *RepositoriesService) ListParticipation(owner, repo string) (*RepositoryParticipation, *Response, error) {
Expand Down Expand Up @@ -185,6 +190,12 @@ type PunchCard struct {

// ListPunchCard returns the number of commits per hour in each day.
//
// If this is the first time these statistics are requested for the given
// repository, this method will return an *AcceptedError and a status code of
// 202. This is because this is the status that github returns to signify that
// it is now computing the requested statistics. A follow up request, after a
// delay of a second or so, should result in a successful request.
//
// GitHub API Docs: https://developer.github.com/v3/repos/statistics/#punch-card
func (s *RepositoriesService) ListPunchCard(owner, repo string) ([]*PunchCard, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/stats/punch_card", owner, repo)
Expand Down