-
-
Notifications
You must be signed in to change notification settings - Fork 251
feat: stream swap quotes using server-sent events #6760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@metamaskbot publish-preview |
Warning MetaMask internal reviewing guidelines:
|
@metamaskbot publish-preview |
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
45b6fbd
to
cfbd89b
Compare
cfbd89b
to
f3be855
Compare
1e904bd
to
6ae7634
Compare
6ae7634
to
bcf5a41
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
@SocketSecurity @microsoft/[email protected] |
0eaa63a
to
9ee0ebb
Compare
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
); | ||
expect(bridgeController.state.quotesLastFetched).toBeNull(); | ||
expect(bridgeController.state.quotesLastFetched).toBeCloseTo( | ||
Date.now() - 1000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the 1000
, 500
, and 10000
values set arbitrarily, and could they affect the output of these test cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're not arbitrary and can affect some of the tests. Most of the tests advance to the next timer (ignores the number of seconds). But some tests that check for intermediate and loading states advance to specific timestamps, which depend on the timeouts set when mocking the server
serverEventHandlers.onValidQuoteReceived(quoteResponse).then((v) => { | ||
return v; | ||
}); | ||
} catch (error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: SSE Handler Fails on Malformed JSON and Unhandled Promises
The fetchBridgeQuoteStream
's onMessage
handler has two issues. JSON.parse(event.data)
lacks error handling, which can crash the SSE connection with malformed JSON. Additionally, the async serverEventHandlers.onValidQuoteReceived
call is not awaited or caught, leading to unhandled promise rejections and potential silent failures.
Explanation
This PR introduces server‑sent events quote streaming and integrates incremental quote updates into the bridge controller polling flow using the
@microsoft/fetch-event-source
libraryhandleQuoteStreaming
handler that callsgetQuoteStream
when thesseEnabled
flag is enabled in LaunchDarklyNote: clients need to patch the
@microsoft/fetch-event-source
dependency such that it rejects instead of resolving when the quote request is cancelled. This preserves the controller's expected request cancellation behaviorOther changes
Client integration PRs
References
Fixes: https://consensyssoftware.atlassian.net/browse/SWAPS-3026
Related to https://github.com/consensys-vertical-apps/va-mmcx-bridge-api/pull/517
Checklist
Note
Introduce server‑sent events quote streaming with incremental updates, refactor fee/sorting logic, tighten error/abort handling, and adjust fetch interfaces; add @microsoft/fetch-event-source.
sseEnabled
; integrate into polling with incrementalonmessage
handling viafetchBridgeQuoteStream
and earlyquotesInitialLoadTime
update.quotes
on new streams; setquotesLastFetched
at start; incrementquotesRefreshCount
post-process; improved error messages and abort handling;resetState(reason)
.AbortSignal
to asset price fetches; guard balance checks with try/catch; keep Solana min-balance fetch async.utils/quote-fees.appendFeesToQuotes
and sorting toutils/quote.sortQuotes
.utils/snaps.getMinimumBalanceForRentExemptionInLamports
.utils/fetch
: factorformatQueryParams
; addfetchBridgeQuoteStream
; drop client-specific cache options forgetQuote
and spot-prices; allowFetchFunction
(RequestInfo|URL|string)
.sseEnabled
toPlatformConfigSchema
; enhanceBridgeControllerState
docs; minor type imports (e.g.,InternalAccount
,BridgeClientId
).@microsoft/fetch-event-source@^2.0.1
; includetests
intsconfig
.Written by Cursor Bugbot for commit d35699b. This will update automatically on new commits. Configure here.