Fix flush() to retry fetch() on transient errors #9
Conversation
| const fetch = this.#fetch; | ||
| return fetch(request); | ||
| } catch (error: any) { | ||
| if (isRetryableError(error)) { |
There was a problem hiding this comment.
Will that really solve the issue?
Don't you need to in the catch of the promise here
hrana-client-ts/src/http/stream.ts
Line 350 in 5d5bcdf
Unless fetch throws that error synchronously (I kinda doubt it does), I think it will escape your solution.
There was a problem hiding this comment.
@mateusfreira I think that point is too late to safely retry because it covers things like failing to process the response. For example, we may have successfully executed a INSERT statement and committed it to the database, but fail later on. Retrying would mean a duplicate insert.
There was a problem hiding this comment.
@mateusfreira I did switch to catching errors from the promise returned by fetch(), though, and added exponential backoff.
This changes flush() to retry the fetch() call on transient errors such as EPIPE, ECONNREFUSED, and ECONNRESET.
This changes flush() to retry the fetch() call on transient errors such as EPIPE, ECONNREFUSED, and ECONNRESET.