retry status 302 does not retry requests

[gajus]

Describe the bug

• Node.js version: v15
• OS & version: macOS Big Sur

Actual behavior

got makes request only once.

In the case of provided code example, "OK" is printed only once.

Expected behavior

got should make request 10 times.

Code to reproduce

const response = await got('http://127.0.0.1:3001/test', {
  followRedirect: false,
  retry: {
    limit: 10,
    statusCodes: [
      302,
    ],
  },
});

You will also need a simple HTTP route, e.g.

router.use('/test', (incomingMessage, serverResponse) => {
  console.log('OK');

  serverResponse.status(302).end('');
});

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

[mindnuts]

@gajus As suggested by the documentation you need to set something like this explicitly:

retry: {
    limit: 5,
    methods: ["POST"]
}

[gajus]

@mindnuts I don't understand what you are implying. As shown in my code example, I did set which status code should be retried.

[mindnuts]

@mindnuts I don't understand what you are implying. As shown in my code example, I did set which status code should be retried.

My bad, i thought you were making a "POST" request, and they are not retried by default.

[gajus]

Is this now a confirmed bug?

[szmarczak]

RFC 7231             HTTP/1.1 Semantics and Content            June 2014

6.4.3.  302 Found

   The 302 (Found) status code indicates that the target resource
   resides temporarily under a different URI.  Since the redirection
   might be altered on occasion, the client ought to continue to use the
   effective request URI for future requests.

   The server SHOULD generate a Location header field in the response
   containing a URI reference for the different URI.  The user agent MAY
   use the Location field value for automatic redirection.  The server's
   response payload usually contains a short hypertext note with a
   hyperlink to the different URI(s).

      Note: For historical reasons, a user agent MAY change the request
      method from POST to GET for the subsequent request.  If this
      behavior is undesired, the 307 (Temporary Redirect) status code
      can be used instead.

[szmarczak]

Hmm I think it should retry. Lemme see

[szmarczak]

Actually this is not a bug. 302 is not an error code in this case, since you explicitly set the followRedirects option to false. Therefore the request is considered successful. In order to retry, you can use the afterResponse hook.

[szmarczak]

@sindresorhus I'm wondering if we should change this behavior. Or we can just document that successful status codes are ignored and afterResponse hook can be used instead.

[szmarczak]

I'm afraid that if we changed this behavior then it could be misused. It's still possible to throw HTTPError inside an afterResponse hook (which I would prefer).

[szmarczak]

(sorry, attached wrong issue, reopened)

[sindresorhus]

I'm afraid that if we changed this behavior then it could be misused.

I'm concerned about this too. I think we should just clearly document the behavior.