sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.33k stars 938 forks source link

statusCodes does not properly limit which requests are retried #2243

Open Inrixia opened 1 year ago

Inrixia commented 1 year ago

Describe the bug

Setting the statusCodes property on retry should ensure that only responses which match that code are retried. However I am seeing retries for statusCodes not specified.

Actual behavior

Non 429 responses are retried.

Expected behavior

Only 429 responses are retried

Code to reproduce

got.extend({
            retry: {
                limit: 5,
                calculateDelay: ({ attemptCount, error }) => {
                    // Retry after the number of seconds specified in the "retry-after" header
                    const retryAfter = error.response?.headers?.["retry-after"];
                    if (retryAfter !== undefined)
                        return parseInt(retryAfter) * 1000; // Convert to milliseconds
                    // Default retry delay
                    return 1000 * attemptCount;
                },
                statusCodes: [429], // Retry on 429 status code
            },
        })

Checklist

Inrixia commented 1 year ago

The limit variable also does not apply at all!

cillen commented 1 year ago

I have the same issue. My retry config: const retry = { retries: RETRY_ATTEMTS, calculateDelay: () => RETRY_DELAY, statusCodes: [102], errorCodes: ['ECONNRESET'], methods: ['POST'], }

I still get retries on 500 errors for example. Got version: 11.8.6 Node version: 18.12.1

Inrixia commented 1 year ago

The only fix for this so far is to check the status code in the function itself and return nothing (which does not retry) if it's not a code you want.

That's not to say this shouldn't be fixed though.

cillen commented 1 year ago

If you omit the calculateDelay in the retry config, the statusCodes limit works, but the retries limit stil does not apply. const retry = { retries: RETRY_ATTEMTS, // calculateDelay: () => RETRY_DELAY, statusCodes: [102], errorCodes: ['ECONNRESET'], methods: ['POST'], }

cillen commented 1 year ago

Any chance there will be a fix for this any time soon?

Inrixia commented 1 year ago

My original report of this is 3 months old so I'm very doubtful. For now ig just ignore the feature and manually implement it.

cillen commented 1 year ago

Ok bummer. It would have been neat to use the config instead of manually wrap the call. Well well..

diVid3 commented 1 year ago

Same issue here, sigh.

rolu01 commented 2 months ago

Is this issue addressed and fixed? Thanks!