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.

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

Enable retry on non-GET requests? #34

Open
AaronHarris opened this issue Oct 20, 2017 · 3 comments
Open

Enable retry on non-GET requests? #34

AaronHarris opened this issue Oct 20, 2017 · 3 comments

Comments

@AaronHarris
Copy link

AaronHarris commented Oct 20, 2017

Hi,

I was curious why the built-in retry logic only functions for GET requests. Could it be extended so clients could retry for POST requests, or whatever method is used for the request? This is something we have a use case for (yes, I am aware of the dangers in general in retrying a failed POST request, we have a use case where this is desired behavior).

Ie. Would a pull request that modified this line https://github.com/then/then-request/blob/master/src/browser.ts#L65 be accepted

-if (options.retry && method === 'GET') {
+if (options.retry) {
@ForbesLindesay
Copy link
Member

I would be happy to see something that added support for retrying non-get requests, but not by default. i.e. it would need to be something that you passed a separate option for. Perhaps if you passed retry: 'dangerous'. Also, any update to the retry logic would also require a matching pull request to https://github.com/ForbesLindesay/http-basic to keep the server side logic in tact (since this is a universal/isomorphic library).

@AaronHarris
Copy link
Author

Yeah since retry only looks for a boolean or a function, one could pass in a string of 'POST' or 'all'. (or an array of string methods to whitelist this for. However, I think for passing in a function, and giving control to the user, if a function returns true it should retry regardless of request - this would likely be a major version breaking change, which would warrant more discussion. That's why I asked what the motivation for placing this restriction is - if it's just really to prevent users from tripping themselves up. (since each request has to specify the type of request anyway).

Sure, I would ensure that any pull requests would be done for both libraries.

@ForbesLindesay
Copy link
Member

I agree, if specified as a function we should probably retry regardless of method. The rational is because you might wrap this module and decide to enable retries by default, but then unexpectedly receive a POST request and retry that too. e.g.

const request = require('then-request');

function awesomeRequest(method, url, options = {}) {
  return request(method, url, {...options, retry: true});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants