Allow to decide if to retry based on error message

[kramarz]

Is your feature request related to a problem? Please describe.
Getting error like:

{"name":"Error","message":"13 INTERNAL: Received RST_STREAM with code 2 triggered by internal client error: read ECONNRESET","stack":"at Object.callErrorFromStatus (/usr/local/autotrader/app/node_modules/@grpc/grpc-js/build/src/call.js:31:26)"}

I want to be able to retry such error, but not all INTERNAL errors.
Describe the solution you'd like
Allow to specify custom shouldRetryFn function that could return True if error message contains certain phrase.
this function would be part of RetryOptions that you can pass as override to createApiCall
Describe alternatives you've considered

• It is currently possible to retry all errors with INTERNAL code.
• Other solution would be adding separate error code for connection errors to grpc library, which could break existing solutions for others relying on INTERNAL code.
• In streaming calls it is already possible to specify similar shouldRetryFn function. Normal calls should also have such option.

[alexander-fenster]

I feel very strongly that this is a wrong solution for the problem. It will definitely put out the fire, and it will hide the real problem behind those 13 INTERNAL errors.

I don't want to block on this, and I believe the 13 INTERNAL affects not only Node.js libraries, but other languages as well. I suggest discussing this with gRPC team first (Cc: @murgatroid99).

[murgatroid99]

I agree that this is the wrong solution to the problem. The right solution is for the gRPC library to report UNAVAILABLE instead of INTERNAL for these errors, if I can find a way to isolate the correct subset of these errors that are connection errors and not other kinds of internal errors.

[crwilcox]

"gRPC library to report UNAVAILABLE instead of INTERNAL for these errors,"

Isn't this a different error type. While "It is currently possible to retry all errors with INTERNAL code" is stated I am not sure that is true. it is possible the backend had an error after altering state for instance.

[murgatroid99]

I meant "these errors" in a very narrow sense here, referring to the error messages ending with "triggered by internal client error: read ECONNRESET". Those indicate that the connection was dropped while waiting for a response, and UNAVAILABLE is the appropriate code for that.

In fact, I just published @grpc/grpc-js version 1.2.9 with a speculative fix that might make that change I just described. If you update your dependencies you can see if that works.

If that doesn't change anything, setting the environment variables GRPC_TRACE=call_stream and GRPC_VERBOSITY=DEBUG will output some logs including one with details on the error from Node that triggers these errors. Those logs could help improve this solution.

[crwilcox]

Do we know that while waiting for response nothing else occurred? We have traced issues returned under 13 RST_STREAM to be an abort on the backend. In the most recent instance we have found cases where it would be accurate to report UNAVAILABLE, I am just not sure how generally that could be applied.

If we can be sure that no change occurred as part of the request, making it clear it is safe for a user to retry would be very helpful.

[murgatroid99]

No, you can't be sure about that here. This error happens when you lose the connection to the server. By definition, you don't know what the server did after that.