RetryAgent - Always Throws Error on Status Codes

[sydo26]

This would solve...

Avoid losing the response body when a status code defined in the RetryAgent is returned after retry attempts are exhausted. Instead of throwing a RequestRetryError immediately after checking if the statusCode is in the list, the agent could just let the request finish, allowing the response body to be read.

The implementation should be like this...

Introduce a new option, for example: noThrowStatusCodes, which accepts an array of status codes.

If the response status code matches any code in this array, the agent would not throw a RequestRetryError and would continue, allowing the response body to be processed.

const retryAgent = new RetryAgent({
    // Continue the request without throwing an error if the cause is a 503 status code.
    // But throw an error if it's a 500 status code and retry attempts are finished
    noThrowStatusCodes: [503],
    statusCodes: [503, 500],
})

I also considered...

Adding the body directly to the RequestRetryError, but it didn't seem like a good idea.

Context

My use case involves scraping data from web applications standardized by a website builder. There are thousands of these applications with the same format.

In some cases, this application returns a 500+ status code, even though the response body contains valid data that I want to capture. This inconsistency comes from the website builder model, but it's something I need to deal with.

I had been using the RetryAgent for a while, but recently I started facing issues when the website builder began returning 500+ status codes on some requests, causing real impacts on my workflow.

Showcase

In the showcase, I have more details to explain the problem. Feel free to access the repository and check the code.

https://github.com/sydo26/retry-agent-always-throws-error-showcase

Final considerations

If there is any other alternative to solve the problem besides the proposed one, please let me know. I am open to suggestions.

I saw this issue opened (November 2024) https://github.com/nodejs/undici/issues/3837, and it seemed similar to the problem I am facing but with a different solution that does not apply to my case.

[sydo26]

I implemented this in a fork. Tomorrow, I'll create the PR and wait for feedback from someone else to see if it's necessary to submit it.

I ran several tests, and it worked. I also wrote a unit test, but I still need to confirm whether this will interfere with retryFn.

[metcoder95]

Thanks for the report and the work already done!

I believe that for handling these cases, the best is to use the retry callback to decide wether or not to retry; adding a flag notThrow could also be a good addition, in this case the agent does not throw upon exhausting attempts but rather return the Response as Body to whatever has been buffered already.

Not convinced adding the notThrowStatusCodes could be the best way to go if retry callback already exists.

[sydo26]

Can you make it work for my use case using only retryFn? If so, that would solve my problem. I haven't been able to reach the same step yet. I might have misunderstood something about this callback.

[sydo26]

Thanks for the report and the work already done!

I believe that for handling these cases, the best is to use the retry callback to decide wether or not to retry; adding a flag notThrow could also be a good addition, in this case the agent does not throw upon exhausting attempts but rather return the Response as Body to whatever has been buffered already.

Not convinced adding the notThrowStatusCodes could be the best way to go if retry callback already exists.

Can you make it work for my use case using only retryFn? If so, that would solve my problem. I haven't been able to reach the same step yet. I might have misunderstood something about this callback.

If you want to test it in the showcase, just add retryFn to sample01: https://github.com/sydo26/retry-agent-always-throws-error-showcase/blob/f84fca3d7eaeb8da76e374927201a1bdf47caea3/src/client.ts#L44

[sydo26]

PR Created 🙏

[metcoder95]

The retryFn will just allow you to say when to retry or not, but won't be able to capture the body in your request, this is for what I suggested another flag that just does not errors out but returns a response instead

[sydo26]

The retryFn will just allow you to say when to retry or not, but won't be able to capture the body in your request, this is for what I suggested another flag that just does not errors out but returns a response instead

Oh, I misunderstood your comment earlier 😅.

I thought you meant that the current retryFn would already solve the issue.

What you said makes sense. When I get a chance, I can review the PR I opened and modify the noThrowStatusCodes option to a new callback that determines whether the error should be thrown or not.

I have a question: is RequestRetryError used only for status code errors? If so, this approach would work well. But if it can also include other types of errors (such as those caused by additional validations after statusCode >= 300), we’d need a way to specifically identify status code errors, like I did in the PR.

[artur-ma]

I think this one should work for you https://github.com/nodejs/undici/pull/4228