feat(loader): add retry logic #12
Conversation
|
@faris-imi can you add some test to this. |
lib/src/loader.ts
Outdated
|
|
||
| const sentry = initSentry(params.sentry); | ||
|
|
||
| return hCaptchaApi(params, sentry).catch( |
There was a problem hiding this comment.
@brdlyptrs rejection is not being caught by try/catch, used .catch() instead
There was a problem hiding this comment.
Thats weird, catch should be the same whether this way or vis try / catch.
There was a problem hiding this comment.
it was due to not awaiting hCaptchaApi before returning function, fixed and reverted try/catch
| message: 'hCaptcha loaded', | ||
| }); | ||
|
|
||
| hCaptchaScripts.push({ promise, scope: frame.window }); |
There was a problem hiding this comment.
@brdlyptrs moved this to resolve block in order to exclude rejected promises from hCaptchaScripts
| }); | ||
|
|
||
| it('should try loading 2 times and succeed on final try', async () => { | ||
| mockFetchScript.mockRejectedValueOnce(SCRIPT_ERROR); |
There was a problem hiding this comment.
| mockFetchScript.mockRejectedValueOnce(SCRIPT_ERROR); |
There was a problem hiding this comment.
How does this work? Is it queuing these?
There was a problem hiding this comment.
Correct, they are queued (latest mock is being called first)
If we remove one rejection mock, it will reject only once (fetchScript will be called twice) meaning test will fail
|
Let's update the README description to include the retry logic as well.
|
lib/src/constants.ts
Outdated
| export const SENTRY_TAG = '@hCaptcha/loader'; | ||
|
|
||
| export const RETRY_COUNT = 2; |
There was a problem hiding this comment.
MAX_RETRIES may sound a bit more descriptive here
lib/src/loader.ts
Outdated
| return promise; | ||
| } catch (error) { | ||
| sentry.captureException(error); | ||
| return Promise.reject(new Error(SCRIPT_ERROR)); | ||
| } | ||
| } | ||
|
|
||
| export async function loadScript(params, retries = 0) { | ||
| const message = retries < RETRY_COUNT ? 'Retry loading hCaptcha Api' : 'Exceeded maximum retries'; |
There was a problem hiding this comment.
we can create the variable areRetriesExceeded and use it in the if below
| @@ -1,6 +1,6 @@ | |||
| # hCaptcha Loader | |||
|
|
|||
| This is a JavaScript library to easily configure the loading of the [hCaptcha](https://www.hcaptcha.com) JS client SDK with built-in error handling. | |||
| This is a JavaScript library to easily configure the loading of the [hCaptcha](https://www.hcaptcha.com) JS client SDK with built-in error handling. It also includes a retry mechanism that will attempt to load the hCaptcha script several times in the event if fails to load due to a network or unforeseen issue. | |||
There was a problem hiding this comment.
It also includes a retry mechanism that will attempt to load the hCaptcha script if it fails to load due to a network or unforeseen issue.
No description provided.