refactor: migrate failover and request utils#554
Conversation
WalkthroughThis change removes local utility implementations for HTTP request handling, failover logic, and timeout management, and replaces their usage throughout the codebase with imports from the external Changes
Estimated code review effort2 (~15 minutes) Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/queries/solverLiquidityQuerires.ts (1)
19-20: Remove the duplicateawait.A double-await is a no-op but adds noise and may mislead readers.
- const response = await await request({ + const response = await request({src/sdk/hyperunit/apis.ts (1)
24-34: Consider surfacing non-2xx HTTP errors for better DX.
response.json()is invoked without checkingresponse.ok. If@defuse-protocol/internal-utils/requestno longer throws on non-2xx (unlike the old local helper), this will result in opaque runtime failures downstream.const response = await request({ ... }) - return response.json() + if (!response.ok) { + // propagate meaningful context to callers + throw new Error( + `generateHLAddress failed: ${response.status} ${response.statusText}`, + ) + } + return response.json() as Promise<GeneratHLAddressResponse>
♻️ Duplicate comments (1)
src/sdk/hyperunit/types.ts (1)
3-6: See earlier note onRequestConfigduplication.Same comment applies here; a shared source of truth would improve maintainability.
🧹 Nitpick comments (2)
src/queries/solverLiquidityQuerires.ts (1)
5-15: File name typo (*Querires*) propagates to importers.The misspelling makes look-ups harder and may break editor tooling. Consider renaming to
solverLiquidityQueries.ts.src/sdk/tokensUsdPricesHttpClient/types.ts (1)
3-6:RequestConfigis duplicated across packages.The same
RequestConfigappears in multipletypes.tsfiles. Consolidate it (e.g. export from@defuse-protocol/internal-utils) to avoid divergence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/constants/nearClient.ts(1 hunks)src/queries/solverLiquidityQuerires.ts(1 hunks)src/sdk/hyperunit/apis.ts(1 hunks)src/sdk/hyperunit/types.ts(1 hunks)src/sdk/tokensUsdPricesHttpClient/apis.ts(1 hunks)src/sdk/tokensUsdPricesHttpClient/types.ts(1 hunks)src/utils/failover.test.ts(0 hunks)src/utils/failover.ts(0 hunks)src/utils/promise/withTimeout.ts(0 hunks)src/utils/request.ts(0 hunks)
💤 Files with no reviewable changes (4)
- src/utils/failover.test.ts
- src/utils/promise/withTimeout.ts
- src/utils/request.ts
- src/utils/failover.ts
🔇 Additional comments (4)
src/queries/solverLiquidityQuerires.ts (1)
1-1: Import switch is correct.Moving to the shared
@defuse-protocol/internal-utilssource is aligned with the refactor goals and keeps a single implementation ofrequest.src/constants/nearClient.ts (1)
1-16: LGTM – import aligns with new failover utility.The switch cleans up local helpers and centralises fail-over logic.
src/sdk/tokensUsdPricesHttpClient/apis.ts (1)
8-15: Order of spreads enforcesGET– intentional?
method: "GET"comes after spreadingconfig?.fetchOptions, overriding any caller-supplied method. Confirm this is desired; otherwise reverse the order.- fetchOptions: { - ...config?.fetchOptions, - method: "GET", - }, + fetchOptions: { + method: "GET", + ...config?.fetchOptions, + },src/sdk/hyperunit/apis.ts (1)
1-1: Externalisingrequestintroduces version-coupling risk—pin dependency & align types.Switching from a local helper to
@defuse-protocol/internal-utilsmakessdktransitively dependent on that package’s release cycle and type definitions.
Please verify:
- The package is already declared in
package.jsonwith an explicit, non-floating version to avoid accidental breaking upgrades.- The exported signature of
requestis stillPromise<Response>(or throws) so the subsequentresponse.json()call remains type-sound.
* beta: refactor: migrate runtime and waitForIntentSettlement utils (#553) refactor: migrate failover and request utils (#554) refactor: migrate publishIntent(s) function (#552) refactor: migrate logger from internal-utils (#551) refactor: migrate poaBridge utils (#550) refactor: use toError and NEP-141 storage utils from internal-utils (#548)
|
🎉 This PR is included in version 1.0.0-beta.204 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Depends on defuse-protocol/sdk-monorepo#41
Summary by CodeRabbit
Refactor
Chores