-
Notifications
You must be signed in to change notification settings - Fork 25
fix: web-usb retry with 5 times #614
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
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughRelease version bumps from 1.1.19-alpha.2 → 1.1.19-alpha.3 across many packages. Core WebUSB handling changed: Changes
Sequence DiagramsequenceDiagram
participant App
participant ensureConnected
participant initDevice
participant Device
participant PermissionPrompt
participant ErrorHandler
rect rgb(245,250,255)
note right of ensureConnected: WebUSB-specific retry state tracked\n(prompt only after max retries)
end
App->>ensureConnected: start connection
ensureConnected->>initDevice: init device (no immediate prompt for WebUSB)
initDevice->>ensureConnected: throw if WebUSB device missing
loop retry attempts
ensureConnected->>Device: attempt connect
alt Device returns WebUSB-not-found
Device-->>ensureConnected: WebDeviceNotFoundOrNeedsPermission
ensureConnected->>ensureConnected: mark WebUSB retry state
else Other error
Device-->>ensureConnected: other error
end
end
alt WebUSB retry state at max attempts
ensureConnected->>PermissionPrompt: optionally post WEB_DEVICE_PROMPT_ACCESS_PERMISSION
PermissionPrompt-->>ensureConnected: response
ensureConnected->>ErrorHandler: reject WebDeviceNotFoundOrNeedsPermission
else other error at max attempts
ensureConnected->>ErrorHandler: reject DeviceNotFound
end
ErrorHandler-->>App: final error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration 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 Disabled knowledge base sources:
📒 Files selected for processing (1)
⏰ 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). (2)
Comment |
ed60155 to
100af92
Compare
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/core/index.ts (1)
814-817: Remove the assignment to unused variable.Line 816 sets
isWebUsbRetrying = true, but this variable is never read. The retry logic works correctly without it—you checkDataManager.isWebUsbConnect(env)at line 849 instead.Remove the assignment:
if (error.errorCode === HardwareErrorCode.WebDeviceNotFoundOrNeedsPermission) { - // WebUSB device not found or needs permission, mark and continue retrying (consistent with Bridge behavior) - isWebUsbRetrying = true; + // WebUSB device not found or needs permission, continue retrying (consistent with Bridge behavior) } else if (
📜 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
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/core/src/core/index.ts(3 hunks)
⏰ 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). (2)
- GitHub Check: lint (22)
- GitHub Check: build (22)
🔇 Additional comments (1)
packages/core/src/core/index.ts (1)
848-856: Logic looks good.The WebUSB retry flow is correct: retry up to 5 times, then show the permission prompt only for WebUSB connections. This aligns with the PR objective.
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.
Actionable comments posted: 1
📜 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
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/core/src/core/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/core/index.ts (5)
packages/core/src/utils/logger.ts (1)
error(50-57)packages/shared/src/HardwareError.ts (1)
HardwareErrorCode(77-476)packages/core/src/data-manager/DataManager.ts (1)
DataManager(60-459)packages/core/src/events/ui-request.ts (2)
createUiMessage(200-205)UI_REQUEST(8-48)packages/core/src/constants/ui-request.ts (1)
UI_REQUEST(3-3)
⏰ 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). (3)
- GitHub Check: lint (22)
- GitHub Check: build (22)
- GitHub Check: Analyze (javascript-typescript)
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.