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

A request should not be retried due to errors in these "upstream codes" #539

Closed
doctorpangloss opened this issue Mar 23, 2022 · 6 comments
Closed

Comments

@doctorpangloss
Copy link

const upStreamCodes = [2, 4, 8, 9, 10, 13, 14, 15];

The error should be propagated to onError and the client should decide what to do

@doctorpangloss
Copy link
Author

To clarify my interpretation of the spec is sending these errors closes the stream. It isn't within spec that then the client retries the same exact request. It was a surprise giving me trouble for a while.

@stephenh
Copy link
Owner

@doctorpangloss yeah, I agree it sounds like this behavior should not be hard-coded, with the disclaimer IANAE an expert on grpc-web / nor use it personally, so ts-proto has had to rely on users incrementally improving the grpc-web-specific output as their individual use cases have needed.

It'd be great great if you want to submit a PR that makes sense defaults configurable, with like a runtime constructor arg or something like that.

Thanks!

@snyh
Copy link

snyh commented Jun 22, 2022

only the status code 14 (UNAVAILABLE) may be retry automatically , all other status code hasn't any sense to retry.

the code

            } else if (upStreamCodes.includes(code)) {
              setTimeout(upStream, DEFAULT_TIMEOUT_TIME);

should be deleted directly.

@stephenh
Copy link
Owner

@snyh I don't personally use this code, but if you want to submit a PR that makes this behavior configurable, with what you think are more sane defaults, that'd be great.

@hersentino
Copy link
Contributor

@stephenh I submitted a PR

@stephenh
Copy link
Owner

Fixed in #618 , thanks @hersentino !

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

4 participants