-
Notifications
You must be signed in to change notification settings - Fork 327
fix(mobula-state): gracefully handle unresolvable symbols #4345
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
base: main
Are you sure you want to change the base?
Conversation
- Change getAssetId and getQuoteId to return undefined instead of throwing errors for unresolvable symbols - Update subscribeMessage builder to skip subscriptions that can't be resolved, logging a warning instead - Fix mock server in tests to handle undefined messages (when subscribeMessage returns undefined) - This prevents the EA from crashing when background cache warming encounters symbols not in includes.json - Other valid subscriptions will continue to work even if some symbols can't be resolved
🦋 Changeset detectedLatest commit: 6382f84 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // Check if we successfully resolved both IDs | ||
| if (assetId === undefined || quoteId === undefined) { | ||
| // Log warning but don't crash - this allows other subscriptions to continue | ||
| console.warn( |
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.
Use logger instead
| mockWsServer.on('connection', (socket) => { | ||
| socket.on('message', (message) => { | ||
| // Skip if message is undefined (happens when subscribeMessage returns undefined) | ||
| if (!message || message === 'undefined') { |
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.
I think !message is the same as message === 'undefined'
mxiao-cll
left a comment
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.
Add some tests
…equests - Return undefined instead of throwing in getAssetId/getQuoteId for unresolvable symbols - Skip subscriptions gracefully when symbols cannot be resolved - Add requestTransforms to uppercase base/quote and resolve via includes.json - Prevents background handler failures from breaking WebSocket connection - Enables lowercase requests (btc/usd) to work seamlessly This fixes the staging issue where external processes sending malformed/mixed-case symbols (e.g. USDtb) caused the background subscription handler to throw errors, preventing the WebSocket connection from being established and causing all subsequent requests to fail with 504 timeouts.
Remove manual includes.json parsing in requestTransforms since the framework already handles this automatically. Just uppercase the parameters and let the framework apply includes.json transformations. This removes 31 lines of redundant code.
- Use context.logger.warn instead of console.warn - Simplify undefined check in fixtures.ts (!message vs !message || message === 'undefined')
6831d2b to
2eb6146
Compare
- BTC is not in includes.json as a base asset - Changed lowercase test to use 'ezeth' which is in includes.json - Changed recovery test to use 'RSETH' which is in includes.json - Added RSETH mock data to fixtures
06bb044 to
52bbd09
Compare
c65de55 to
30349c0
Compare
…id cache collision
| transport: 'ws', | ||
| }) | ||
| await testAdapter.waitForCache(8) // Wait for all primed pairs to be cached (7 new + 1 initial = 8 total, gho/usd doesn't create a new entry since it uppercases to GHO/USD) | ||
| }, 30000) |
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.
It should still work without the 30000
Closes #ISSUE_NUMBER_GOES_HERE
Description
fix for pre release mobula-state v2 EA
Changes
Steps to Test
Quality Assurance
infra-k8sconfiguration file.adapter-secretsconfiguration file.test-payload.jsonfile with relevant requests.feature/x,chore/x,release/x,hotfix/x,fix/x) or is created from Jira.