Fix/execute with retry wrapper exec #479
Conversation
…execute-with-retry-wrapper-exec
packages/misc/src/lib/misc.ts
Outdated
| import Ajv, { JSONSchemaType } from 'ajv'; | ||
| //@ts-ignore no tyoe df | ||
| import fetchRetry from 'fetch-retry'; |
There was a problem hiding this comment.
Hmm I'm worried how browser (react, vite, parcel, etc.) would react to this dependency, I'll give it a spin next week
There was a problem hiding this comment.
its bundle is 'umd' format and does not attempt to redefine 'fetch' globally. I think it's okay but I'll publish a testing build with it for us to test with more before committing to it.
…execute-with-retry-wrapper-exec
…ry-fetch` has its own unit and integration tests
MaximusHaximus
force-pushed
the
fix/execute-with-retry-wrapper-exec
branch
from
ad190c1 to
c6a3768
Compare
…-nodejs`. Fix return type on `sendRequest`; it doesn't return a Response. Throw in promise chain instead of creating extra rejected promises
MaximusHaximus
force-pushed
the
fix/execute-with-retry-wrapper-exec
branch
from
c6a3768 to
f168848
Compare
| // error: Error | null, | ||
| // response: Response | null | ||
| ) { | ||
| return Math.random() * (Math.pow(2, attempt) * interval); |
There was a problem hiding this comment.
Is this truly exponential? since we are multiplying by a random value couldn't we end up with a lower interval?
…Protocol/js-sdk into fix/execute-with-retry-wrapper-exec
|
|
|
Closing for #497 |
Description
Removes
executeWithRetryas we no longer need to retry operations due to network stability fixes. We now will only retry operations which are critical to the standup of the sdk. The included node endpoints which are eligible for retry at this time are:/handshakeStatus codes we will retry on:
Introduces a new package fetch-retry which now handles retry logic for single requests. As we no longer need to group requests and retry as single network operation.
Type of change
How Has This Been Tested?
Tested with local testing to force endpoint failure to observe retry.
Checklist: