fix: increase timeout to 60 seconds for dump ui on ios #131
Conversation
WalkthroughIntroduces configurable timeout handling for HTTP requests. An internal method Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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
🧹 Nitpick comments (3)
devices/wda/types.go (1)
24-28: Client timeout increase aligns with longer UI dump operationsBumping the
http.Clienttimeout to 60s is consistent with the new 60s /source calls and prevents the client-level timeout from aborting long-running dumps prematurely; change looks good. If more timeout variants are added later, consider centralizing them as namedtime.Durationconstants for clarity.devices/wda/source.go (1)
6-7: Using 60s timeout for source fetches matches the PR intentRouting
GetSource/GetSourceRawthroughgetEndpointWithTimeout(..., 60*time.Second)cleanly targets longer timeouts at the UI dump without changing other callers. You might optionally extract aconst sourceTimeout = 60 * time.Secondto avoid repeating the literal and make the intent self-documenting.Also applies to: 80-89, 92-100
devices/wda/requests.go (1)
65-67: GetEndpoint wrapper preserves existing behavior; consider sharing the 5s defaultHaving
GetEndpointdelegate togetEndpointWithTimeout(endpoint, 5*time.Second)keeps the public API and effective timeout the same while enabling longer timeouts for specific endpoints. Since5*time.Secondis also used in POST/DELETE, you could introduce aconst defaultTimeout = 5 * time.Secondto avoid magic numbers and keep these call sites in sync.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
devices/wda/requests.go(2 hunks)devices/wda/source.go(3 hunks)devices/wda/types.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
devices/wda/requests.go (1)
devices/wda/types.go (1)
WdaClient(10-15)
⏰ 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). (1)
- GitHub Check: lint
🔇 Additional comments (1)
devices/wda/requests.go (1)
27-63: Configurable GET helper is implemented correctly
getEndpointWithTimeoutcleanly encapsulates the GET flow with a per-call timeout, including status checks and JSON parsing, and is suitable for both short (5s) and long (60s) scenarios. Given that this now centralizes GET behavior, future adjustments to logging or error handling will be straightforward here.
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.