fix: premount loader race condition #896
Conversation
ArushKapoorJuspay
requested review from
seekshiva,
PritishBudhiraja,
sakksham7 and
aritro2002
as code owners
|
Review changes with |
ArushKapoorJuspay
had a problem deploying
to
Testing
GitHub Actions
Failure
ArushKapoorJuspay
force-pushed
the
fix/pre-mount-loader-race
branch
from
758b6dd to
080eb7b
Compare
ArushKapoorJuspay
had a problem deploying
to
Testing
GitHub Actions
Failure
src/Payments/PreMountLoader.res
Outdated
| @@ -12,30 +12,36 @@ let sendPromiseData = (promise, key) => { | |||
|
|
|||
| let useMessageHandler = getPromisesAndHandler => { | |||
| React.useEffect(_ => { | |||
| let (promises, messageHandler) = getPromisesAndHandler() | |||
| let messageHandler = getPromisesAndHandler() | |||
There was a problem hiding this comment.
This variable used to be called getPromisesAndHandler because it used to return promise and message handler. if that is changed to only return handler, then can we rename the function to reflect that?
There was a problem hiding this comment.
Changed the name
src/hyper-loader/Elements.res
Outdated
| fetchPaymentsList(mountedIframeRef, componentType) | ||
| let paymentMethodsPromise = Promise.make((resolve, _) => { | ||
| fetchPaymentsList(mountedIframeRef, componentType, resolve) | ||
| }) |
There was a problem hiding this comment.
why do we have to wrap fetchPaymentsList by Promise.make((resolve, _) =. {} ? Why can't fetchPaymentsList return a promise on its own and resolve that promise when it wants
Wrapping with Promise.make and passing the resolve function seems unnecessary - adding a lot of responsibility on the function caller. It might be simpler if the function returns promise by itself
There was a problem hiding this comment.
Moved Promise inside the function
| fetchSavedPaymentMethods(mountedIframeRef, false, componentType) | ||
| Promise.make((resolve, _) => { | ||
| fetchSavedPaymentMethods(mountedIframeRef, false, componentType, resolve) | ||
| }) |
There was a problem hiding this comment.
same over here as well. Can fetchSavedPaymentMethods return promise instead of the caller having to do Promise.make and pass resolve function down as arg?
There was a problem hiding this comment.
moved the promise inside here as well
ArushKapoorJuspay
had a problem deploying
to
Testing
GitHub Actions
Failure
PritishBudhiraja
had a problem deploying
to
Testing
GitHub Actions
Failure
PritishBudhiraja
had a problem deploying
to
Testing
GitHub Actions
Failure
PritishBudhiraja
had a problem deploying
to
Testing
GitHub Actions
Failure
github-actions
bot
added
the
Closed
## [0.114.2](v0.114.1...v0.114.2) (2025-02-18) ### Bug Fixes * premount loader race condition ([#896](#896)) ([7c8f1df](7c8f1df))
Type of Change
Description
Hyperswitch Web creates an invisible iframe (Pre Mount Loader) to make API calls for fetching payment methods, customer saved payment methods, and session tokens. The top-level page retrieves these responses via post messages. The iframe includes a cleanup function that waits for all API calls to complete before unmounting itself by sending a post message to the top level.
However, a race condition occurred where the iframe would receive all API responses but unmount before the top level had retrieved them, as the post messages to fetch these responses had not yet been triggered. This led to lost API responses.
To fix this, the iframe now unmounts only after all responses have been successfully received at the top level.
How did you test it?
Test whether SDK is rendering properly with correct data in different browsers with varied network throttling conditions
TrustPay ApplePay also needs to be tested
Checklist
npm run re:build