Fix/allnetwork #540
Fix/allnetwork #540
Conversation
WalkthroughVersion bumps from 1.1.7 to 1.1.8 across many packages. Core change updates allNetworkGetAddressByLoop to pass errors in the final callback. Example app aligns its callbacks: per-item callback drops error; final callback now includes error. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Example App
participant Core as Core allNetworkGetAddressByLoop
participant Dev as Device/Networks
participant LoopCB as onLoopItemResponse(data)
participant FinalCB as onAllItemsResponse(data, error)
App->>Core: invoke allNetworkGetAddressByLoop(...)
Core->>Dev: iterate networks and fetch addresses
loop For each item
Dev-->>Core: item result (data or error internally)
Core-->>LoopCB: data
end
Dev-->>Core: finish (aggregate data, error?)
Core-->>FinalCB: (data, error)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
packages/connect-examples/expo-example/src/components/PlaygroundExecutor.tsx (3)
34-35: Returning a string here drops the message; call onExecute instead.The onPress handler ignores the return value, so users won’t see these messages.
Use onExecute for early exits:
- if (!sdk) return intl.formatMessage({ id: 'tip__sdk_not_ready' }); + if (!sdk) { + onExecute(JSON.stringify({ error: intl.formatMessage({ id: 'tip__sdk_not_ready' }) }, null, 2)); + return; + } ... - if (!selectedDevice) return intl.formatMessage({ id: 'tip__need_connect_device_first' }); + if (!selectedDevice) { + onExecute(JSON.stringify({ error: intl.formatMessage({ id: 'tip__need_connect_device_first' }) }, null, 2)); + return; + } ... - if (!selectedDevice) return intl.formatMessage({ id: 'tip__need_connect_device_first' }); + if (!selectedDevice) { + onExecute(JSON.stringify({ error: intl.formatMessage({ id: 'tip__need_connect_device_first' }) }, null, 2)); + return; + }Also applies to: 75-76, 79-80
68-68: Avoid noisy logs in the example UI.Either remove the console.log or guard it behind a dev flag.
Apply:
- console.log('requestParams: ', requestParams); + if (__DEV__) console.log('requestParams: ', requestParams);
85-87: Harden error reporting.Unknown thrown values may lack message. Fall back to String(error).
Apply:
- } catch (error: any) { - // Adjust according to your error type - onExecute(JSON.stringify({ error: error.message }, null, 2)); + } catch (error: unknown) { + const message = error && typeof error === 'object' && 'message' in error ? (error as any).message : String(error); + onExecute(JSON.stringify({ error: message }, null, 2));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
packages/connect-examples/expo-example/package.json(2 hunks)packages/connect-examples/expo-example/src/components/PlaygroundExecutor.tsx(1 hunks)packages/connect-examples/expo-playground/package.json(2 hunks)packages/core/package.json(2 hunks)packages/core/src/inject.ts(2 hunks)packages/hd-ble-sdk/package.json(2 hunks)packages/hd-common-connect-sdk/package.json(2 hunks)packages/hd-transport-electron/package.json(2 hunks)packages/hd-transport-emulator/package.json(2 hunks)packages/hd-transport-http/package.json(2 hunks)packages/hd-transport-lowlevel/package.json(2 hunks)packages/hd-transport-react-native/package.json(2 hunks)packages/hd-transport-web-device/package.json(2 hunks)packages/hd-transport/package.json(1 hunks)packages/hd-web-sdk/package.json(2 hunks)packages/shared/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/inject.ts (2)
packages/core/src/types/params.ts (1)
Unsuccessful(49-52)packages/core/src/types/api/allNetworkGetAddress.ts (1)
AllNetworkAddress(94-105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: publish-and-release
- GitHub Check: lint (22)
- GitHub Check: build (22)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
packages/hd-transport-react-native/package.json (2)
3-3: Version bump approved.
Matches the monorepo release to 1.1.8.
22-23: Dependency alignment to 1.1.8 confirmed.
Transport and shared are now in lockstep across every package. After you publish, please verify that React Native consumers resolve a single version to prevent duplicate native modules.packages/hd-web-sdk/package.json (2)
3-3: Version bump approved.
No other script or metadata changes. Good.
24-27: Deps synced to 1.1.8 — verify downstream callback signature changesWe found the updated declaration for allNetworkGetAddressByLoop in
• packages/core/src/types/api/allNetworkGetAddress.ts:124No internal calls were detected in this repo. Please check any consumer code that invokes this API to ensure it matches the new signature (returns Response<AllNetworkAddress[]> and takes CommonParams & AllNetworkGetAddressParamsByLoop).
packages/hd-transport/package.json (1)
3-3: Version bump approved.
Transport at 1.1.8 matches dependent packages.packages/connect-examples/expo-playground/package.json (2)
3-3: Version bump approved.
Playground aligns to 1.1.8.
20-22: Deps bumped to 1.1.8 – please verify example callbacks
I couldn’t find any uses of the new loop API or callbacks in the expo-playground. Manually confirm that in your example code you:
- Pass
(data, error)into the finalonAllItemsResponsecallback- Remove the
errorargument from per-item callbacks (e.g.onLoopItemResponse)packages/core/package.json (1)
3-3: Core moved to 1.1.8; deps synced.Looks consistent with the repo-wide bump. Ensure CHANGELOG and release tag reflect the callback signature update (onAllItemsResponse now supports error).
Also applies to: 28-29
packages/hd-transport-lowlevel/package.json (1)
3-3: Transport-lowlevel bump is consistent.Version and internal deps pinned to 1.1.8. No further action from this file.
Also applies to: 22-23
packages/hd-transport-electron/package.json (1)
3-3: Electron transport aligned to 1.1.8.Dependency on hd-shared also updated. Nothing else needed here.
Also applies to: 28-28
packages/connect-examples/expo-example/package.json (1)
3-3: Expo example versions synced to 1.1.8.Good alignment. Since core’s API adds an error in the final callback, confirm the example code uses the new onAllItemsResponse(data, error) signature and still compiles.
Also applies to: 22-25
packages/hd-transport-emulator/package.json (1)
3-3: LGTM on version and dependency bumps.Version and deps are aligned to 1.1.8. Looks consistent.
Also applies to: 27-28
packages/hd-transport-http/package.json (1)
3-3: LGTM on version and dependency bumps.Everything aligns to 1.1.8 as expected.
Also applies to: 27-28
packages/hd-transport-web-device/package.json (1)
3-3: LGTM on version/deps updates.Runtime deps and the electron dev dep are all on 1.1.8. Good.
Also applies to: 23-24, 27-27
packages/hd-common-connect-sdk/package.json (2)
3-3: LGTM on version/dependency alignment.Core, shared, and transports all pinned to 1.1.8. Looks consistent.
Also applies to: 22-29
22-29: Update lingering 1.1.7 referenceWe found one leftover version pin. Please bump it to 1.1.8 and rerun the check.
• packages/connect-examples/electron-example/package.json (line 5)
- "version": "1.1.7", + "version": "1.1.8",Likely an incorrect or invalid review comment.
packages/connect-examples/expo-example/src/components/PlaygroundExecutor.tsx (2)
53-55: Per-item callback shape change is correct.Dropping the error param from onLoopItemResponse matches the new API.
51-59: Verify oldonAllItemsResponsehandlers
- Ripgrep found no single-param assignments in
.ts/.tsx.- Double-check any
.js/.jsxor external modules for calls likeonAllItemsResponse(data).- Ensure every consumer uses the new
(data, error)signature.packages/core/src/inject.ts (2)
5-5: Use a type-only import forUnsuccessfulSwitch to a type-only import to avoid including a runtime symbol and improve tree-shaking.
-import { Unsuccessful } from './types'; +import type { Unsuccessful } from './types';
176-197: AllNetworkGetAddressByLoop callback signature is up-to-date
I’ve confirmed that in packages/core/src/types/api/allNetworkGetAddress.ts, onAllItemsResponse is defined as (data?: AllNetworkAddress[], error?: Unsuccessful). The implementation in inject.ts passes both arguments correctly. No remaining call sites (including examples) need updating.
packages/connect-examples/expo-example/src/components/PlaygroundExecutor.tsx
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Bug Fixes
Chores