Skip to content
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

Jump to bottom

Clarify that automatic network retries occur for most api errors #567

Merged
merged 2 commits into from
Feb 14, 2019
Merged

Clarify that automatic network retries occur for most api errors #567

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1d6530
Clarify that automatic network retries occur for most api errors
rattrayalex-stripe Feb 14, 2019
c72b7f0
Adopt stripe-php's verbiage
rattrayalex-stripe Feb 14, 2019
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ if (process.env.http_proxy) {

### Network retries

Automatic network retries can be enabled with `setMaxNetworkRetries`. This will retry requests `n` times with exponential backoff if they fail due to connection or conflict errors. [Idempotency keys](https://stripe.com/docs/api/idempotent_requests) are added where appropriate to prevent duplication.
Automatic network retries can be enabled with `setMaxNetworkRetries`. This will retry requests `n` times with exponential backoff if they fail due to connection errors or most api errors. [Idempotency keys](https://stripe.com/docs/api/idempotent_requests) are added where appropriate to prevent duplication.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "most api errors" imply? It seems wrong. The most important API errors are card_error and invalid_request_error which we would explicitly never retry right?

Copy link
Contributor

@paulasjes-stripe paulasjes-stripe Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'd be more explicit here and list the exact errors we do don't retry on to reduce any confusion.

Copy link
Contributor Author

@rattrayalex-stripe rattrayalex-stripe Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

to wit:

Automatic network retries can be enabled with setMaxNetworkRetries. This will retry requests n times with exponential backoff if they fail due to connection errors, 409 (conflict) errors, 503 (availability) errors, and 500 (internal) errors on GET/DELETE. Idempotency keys are added where appropriate to prevent duplication.

400 (user) errors and 429 (rate limit exceeded) errors are not retried, nor are 500 errors which result from POST requests, as our idempotency framework would simply no-op and return a replayed 500 error.

eh?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would personally remove the last sentence, it seems quite confusing. Or we need to get into the weeds of what is retry-able and what isn't which seems out of scope from this small section in the readme


```js
// Retry a request once before giving up
Expand Down