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

RetryAgent - Always Throws Error on Status Codes #4080

Open
sydo26 opened this issue Mar 3, 2025 · 7 comments
Open

RetryAgent - Always Throws Error on Status Codes #4080

sydo26 opened this issue Mar 3, 2025 · 7 comments
Labels
enhancement New feature or request

Comments

@sydo26
Copy link

sydo26 commented Mar 3, 2025

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) #3837, and it seemed similar to the problem I am facing but with a different solution that does not apply to my case.

@sydo26 sydo26 added the enhancement New feature or request label Mar 3, 2025
@sydo26 sydo26 changed the title RetryAgent RetryAgent - Always Throws Error on Status Codes Mar 3, 2025
@sydo26
Copy link
Author

sydo26 commented Mar 4, 2025

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
Copy link
Member

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
Copy link
Author

sydo26 commented Mar 4, 2025

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
Copy link
Author

sydo26 commented Mar 4, 2025

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
Copy link
Author

sydo26 commented Mar 5, 2025

PR Created 🙏

@metcoder95
Copy link
Member

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
Copy link
Author

sydo26 commented Mar 5, 2025

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.

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

No branches or pull requests

2 participants