fix(perps): prevent CLIENT_NOT_INITIALIZED during reconnection and cold start - #9032
Merged
Merged
Conversation
…ld start - Add `#getActiveProviderWhenReady()` that awaits in-flight initialization before returning the provider, so trading actions (placeOrder, editOrder, cancelOrder, closePosition, deposit, withdraw, etc.) wait for cold-start or reconnection to finish instead of failing immediately - Always throw the plain `CLIENT_NOT_INITIALIZED` code from `getActiveProvider()` instead of a compound string that breaks i18n lookup - Extract shared `#createAllClients()` in HyperLiquidClientService so both `initialize()` and `#handleConnectionDrop()` recreate all four SDK clients (including ExchangeClient and HTTP InfoClient), fixing `isInitialized()` returning false after WebSocket reconnection - Move provider lookup inside the try block in `depositWithConfirmation` to preserve stale deposit cleanup on initialization failure
michalconsensys
temporarily deployed
to
default-branch
June 8, 2026 07:51 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
michalconsensys
changed the title
oxfmt and changelog validation both require a single blank line before the next version heading.
4 tasks
abretonc7s
requested changes
Aug 13, 2026
abretonc7s
left a comment
abretonc7s
left a comment
Contributor
There was a problem hiding this comment.
Bug still exist it just moves it
## Explanation Stacked on #9032 — merge that first, this targets its branch. #9032 makes trading actions wait for an in-flight `PerpsController.init()` instead of failing immediately. Running that branch against HyperLiquid testnet shows the wait works but the action still fails: it now reaches `HyperLiquidProvider.placeOrder`, which resolves asset info **before** it ensures trading readiness. That path calls `#getCachedMeta` → `#clientService.getInfoClient()` → `ensureInitialized()`, which throws `CLIENT_NOT_INITIALIZED` while the SDK clients are still down. `#ensureReadyForTrading()` (and with it `#ensureClientsInitialized()`) only runs later in `placeOrder`. So on #9032 alone the caller stops getting a thrown error and starts getting `{ success: false, error: 'CLIENT_NOT_INITIALIZED' }` — same failure, quieter. This adds the missing ensure on the cache-miss path of `#getCachedMeta`, which is the first client touch on the write path. It is idempotent, and a warm cache hit returns before it, so an initialized provider does no extra work. All 12 `#getCachedMeta` callers benefit. ## Proof Real `PerpsController` on HyperLiquid **testnet** (headless core adapter: real messenger wiring, fixture signer, real transports). Scenario: `disconnect()` → start `init()` without awaiting → `placeOrder` immediately. Order is a resting limit 50% below market (never fills), cancelled by the rig. | Build | Cold-start result | | --- | --- | | `main` | rejects, thrown `CLIENT_NOT_INITIALIZED` | | #9032 alone | resolves `{ success: false, error: 'CLIENT_NOT_INITIALIZED' }` | | #9032 + this change | **order placed** (`57799146376`), cancelled | Executable recipe (`perps-cold-start-order.recipe.json`, core adapter): asserts `verdict: ORDER_PLACED` and `error: null` from the rig. **Fails** on #9032 alone (`cold-start-order.stdout does not contain "verdict": "ORDER_PLACED"`), **passes** with this change. Recipe and rig live in the perps recipe library rather than this repo; happy to share both. Unit guard: `HyperLiquidProvider.trading.test.ts` — "brings the SDK clients up before reading asset metadata" makes the mocked `getInfoClient` throw `CLIENT_NOT_INITIALIZED` until `initialize()` runs, then asserts `placeOrder` succeeds. Red before the change, green after. ## Validation - `jest tests/src/providers` — 14 suites, 667 passed, 0 failed - `eslint` on both changed files — clean - `changelog:validate` — clean ## References * Builds on #9032 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Narrow, idempotent initialization guard on an existing metadata fetch path; warm-cache behavior unchanged and covered by a focused unit test. > > **Overview** > **Fixes a remaining cold-start / post-disconnect `CLIENT_NOT_INITIALIZED` path** on HyperLiquid trading after controller init waits (#9032): `placeOrder` still resolves asset info (via `#getCachedMeta` → `InfoClient`) **before** `#ensureReadyForTrading`, so the first metadata fetch could throw or return `{ success: false, error: 'CLIENT_NOT_INITIALIZED' }` while clients were still down. > > On **cache miss** (or `skipCache`), `#getCachedMeta` now **`await`s `#ensureClientsInitialized()`** before `getInfoClient()`. That call is idempotent; **warm cache hits** return earlier and do not add work. > > Adds a **unit test** that mocks `getInfoClient` to fail until `initialize()` runs, and documents the fix in **CHANGELOG**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 32e3a60. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
abretonc7s
self-requested a review
August 13, 2026 12:18
abretonc7s
previously approved these changes
Aug 13, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
4 tasks
## Explanation Addresses the reconnect-readiness finding raised in review on #9032; stacked on that branch. `#handleConnectionDrop` constructs all four SDK clients *before* `await newWsTransport.ready()`. The catch path left them in place, so a reconnect whose readiness rejected kept `isInitialized()` returning `true`. Callers gated on `isInitialized()` / `ensureInitialized()` then issued WebSocket-backed reads (`getInfoClient`, `getSubscriptionClient`) over a socket that never opened, instead of taking the uninitialized path. Writes were unaffected — `ExchangeClient` runs on the HTTP transport. This clears the clients and both transports on failure, mirroring the cleanup `initialize()` already performs, before the retry is scheduled. The retry then rebuilds them from scratch as it already did. ## Scope of the regression Measured with a rejected `ready()` on the reconnect path: | Scenario | `0837703` (before #9032) | branch tip `322eda6` | with this change | | --- | --- | --- | --- | | plain drop after `initialize()` | `isInitialized()` **true** | **true** | **false** | | after `disconnect()` | `isInitialized()` **false** | **true** | **false** | So the post-disconnect row is the behaviour change introduced by recreating all four clients on the reconnect path; the plain-drop row was already wrong before and is fixed here too. ## Test `HyperLiquidClientService.test.ts` gains two cases under the reconnection describe: readiness rejects after a plain `initialize()`, and after a `disconnect()`. Both assert `isInitialized() === false` (the first also asserts `getSubscriptionClient()` is `undefined`). Both fail on the branch without this change (`Received: true`) and pass with it. ## Validation - `jest tests/src/services tests/src/providers` — 34 suites, 1484 passed, 0 failed - `eslint` on both changed files — clean ## References * Stacked on #9032, follows #9865 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches WebSocket reconnection lifecycle in a critical trading client path; behavior change is narrow (fail-closed on failed ready) but affects when reads proceed after a bad reconnect. > > **Overview** > Fixes a **reconnect** edge case where HyperLiquid SDK clients are built before `WebSocketTransport.ready()` completes. If `ready()` rejects, the catch path used to leave those clients in place, so **`isInitialized()` stayed true** even though the socket never opened—callers gated on initialization could issue WebSocket-backed reads on a dead connection instead of failing closed or retrying. > > On reconnect failure, **`#handleConnectionDrop` now clears all four SDK clients and both transports** (with safe `close()` on the WS transport), matching the cleanup **`initialize()`** already does on failure, before scheduling the existing retry. > > **Tests** cover readiness rejection after a normal `initialize()` and after `disconnect()`; both assert `isInitialized() === false`. Changelog updated under Fixed. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4df3413. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1f4bb69. Configure here.
abretonc7s
enabled auto-merge
August 13, 2026 14:12
aganglada
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
CLIENT_NOT_INITIALIZEDis thrown whenever a user attempts a trading or funding action while HyperLiquid SDK clients are not fully initialised. That covers two scenarios: a cold-start race (user acts beforeinit()completes) and a mid-session WebSocket drop (reconnection clears SDK client references). In both cases the action fails immediately with no wait/retry. On failed init, the error string is also malformed (CLIENT_NOT_INITIALIZED: <reason>), which breaks i18n lookup and surfaces a generic "Unknown error".This PR fixes three root causes in
@metamask/perps-controller:No wait-for-ready on action calls — Trading methods (
placeOrder,editOrder,cancelOrder,closePosition,deposit,withdraw, etc.) calledgetActiveProvider(), which throws immediately wheninitializationState !== Initialized. They now use#getActiveProviderWhenReady(), which awaits the in-flight#initializationPromisewhen state isInitializing, then resolves the provider. Synchronous callers that need fail-fast behaviour still usegetActiveProvider()directly.Incomplete client recreation on reconnect —
#handleConnectionDrop()only recreated the WebSocket-backed#infoClientand#subscriptionClient, leaving#exchangeClientand#infoClientHttpasundefined. BecauseisInitialized()requires all four clients, write operations stayed broken after reconnect. Wallet params are now stored in#walletParamsduringinitialize(), and a shared#createAllClients()recreates all four SDK clients on both init and reconnect.Compound error string breaks translation —
getActiveProvider()now always throws and records the plainCLIENT_NOT_INITIALIZEDcode (no: <reason>suffix), so client i18n lookup by error code works again.Also moves provider lookup in
depositWithConfirmationonto the ready path inside the existingtry, so a failed init still hits the catch cleanup for stale deposit state.References
Checklist
Note
High Risk
Changes initialization gating, WebSocket reconnect publishing, and all major trading/deposit/withdraw entry points—errors here could block orders or allow races during reconnect.
Overview
Fixes
CLIENT_NOT_INITIALIZEDwhen users trade or move funds during cold start or while the HyperLiquid WebSocket is reconnecting.Controller: Async trading and funding paths (
placeOrder,cancelOrder,closePosition,depositWithConfirmation,withdraw, etc.) now use#getActiveProviderWhenReady(), which awaits in-flight init instead of failing immediately.getActiveProvider()always throws the plainCLIENT_NOT_INITIALIZEDcode (no: reasonsuffix) so i18n lookup works.HyperLiquid client layer: Wallet params are stored and
#createAllClients/#createHttpClientsrecreate all four SDK clients on init and reconnect. On reconnect, WebSocketInfoClient/SubscriptionClientare cleared untiltransport.ready()succeeds; HTTPInfoClientandExchangeClientstay available during retries.getInfoClient({ useHttp: true })is used for metadata/spot REST paths;ensureSubscriptionClientavoids competinginitialize()during reconnect backoff.Provider:
getCachedMetaawaits#ensureClientsInitialized()before the first metadata read on the order path; price fallback uses HTTP info client.Reviewed by Cursor Bugbot for commit c932cac. Bugbot is set up for automated code reviews on this repo. Configure here.