Skip to content

fix(sdk): add optional chaining to getSpotMarketAccountAndSlot calls - #2176

Closed
claygeo wants to merge 1 commit into
velocity-exchange:masterfrom
claygeo:fix/optional-chaining-spot-market
Closed

fix(sdk): add optional chaining to getSpotMarketAccountAndSlot calls#2176
claygeo wants to merge 1 commit into
velocity-exchange:masterfrom
claygeo:fix/optional-chaining-spot-market

Conversation

@claygeo

@claygeo claygeo commented Apr 4, 2026

Copy link
Copy Markdown

Summary

Closes #2137

getSpotMarketAccount() and forceGetSpotMarketAccount() access .data on the return value of getSpotMarketAccountAndSlot() without optional chaining. If the market index doesn't exist or the account subscriber hasn't loaded yet, this throws a runtime error.

Fix

// Before: crashes if result is undefined
this.accountSubscriber.getSpotMarketAccountAndSlot(marketIndex).data;

// After: returns undefined safely
this.accountSubscriber.getSpotMarketAccountAndSlot(marketIndex)?.data;

Both occurrences fixed:

  • getSpotMarketAccount() (line 699)
  • forceGetSpotMarketAccount() (line 710)

The return type is already SpotMarketAccount | undefined, so callers already handle the undefined case.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling in spot market account retrieval methods to safely handle missing subscriber data without throwing exceptions, making the SDK more resilient.

getSpotMarketAccount() and forceGetSpotMarketAccount() access .data
on the return value of getSpotMarketAccountAndSlot() without optional
chaining. If the market index doesn't exist or the subscriber hasn't
loaded yet, this crashes with "Cannot read properties of undefined."

Closes #2137
@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 00d0dcae-3107-458f-8101-7a3bca4056d5

📥 Commits

Reviewing files that changed from the base of the PR and between 0ae3e3b and ceaa9ff.

📒 Files selected for processing (1)
  • sdk/src/driftClient.ts

Walkthrough

Updated DriftClient.getSpotMarketAccount() and forceGetSpotMarketAccount() methods to use optional chaining when accessing spot market subscriber data, preventing potential runtime errors when subscriber lookups return undefined values.

Changes

Cohort / File(s) Summary
Optional Chaining Safety Fix
sdk/src/driftClient.ts
Added optional chaining (?.) to getSpotMarketAccountAndSlot(marketIndex)?.data calls in both getSpotMarketAccount() and forceGetSpotMarketAccount() methods to gracefully handle cases where subscriber data is unavailable, returning undefined instead of throwing errors.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A rabbit hops through safer code,
With question marks to light the road,
No more crashes in the night,
Optional chains make all things right! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding optional chaining to getSpotMarketAccountAndSlot calls in the SDK to handle missing spot market data safely.
Linked Issues check ✅ Passed The PR directly addresses issue #2137 by implementing optional chaining in both getSpotMarketAccount and forceGetSpotMarketAccount methods as requested.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue; only two method call sites in driftClient.ts were modified to add optional chaining.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claygeo

claygeo commented Jun 19, 2026

Copy link
Copy Markdown
Author

Friendly nudge on this one, it still applies cleanly to master and is a small null-safety fix.

It adds optional chaining to the two getSpotMarketAccountAndSlot(marketIndex).data reads in getSpotMarketAccount() / forceGetSpotMarketAccount(). Both methods declare a SpotMarketAccount | undefined return and getSpotMarketAccountAndSlot is typed DataAndSlot<SpotMarketAccount> | undefined, so for a missing or not-yet-loaded market index the current .data throws Cannot read properties of undefined instead of returning undefined as the signature promises (there's no strictNullChecks in the SDK tsconfig to catch it).

It's effectively a regression fix: the optional chaining lived here until #1885 collapsed it back to .data, so this just restores the prior guard. It also matches the perp accessors (getPerpMarketAccount), which already use ?.data.

Still MERGEABLE / CI green with no CodeRabbit findings. A few later PRs target the same issue (#2186, #2188, #2191); this is the earliest, so happy to consolidate here, or to defer if you'd prefer one of those. Closes #2137. Could a maintainer take a quick look when there's a moment? Happy to rebase if it helps.

cc @lowkeynicc

@claygeo claygeo closed this by deleting the head repository Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Minor driftClient improvement

1 participant