Skip to content

fix: resolve final 4 bugs before convergence#81

Merged
GT-610 merged 1 commit into
mainfrom
final-code-fix
Jun 10, 2026
Merged

fix: resolve final 4 bugs before convergence#81
GT-610 merged 1 commit into
mainfrom
final-code-fix

Conversation

@GT-610

@GT-610 GT-610 commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Bug fixes:

  • Aria2RpcClient.close(): completeError all pending completers with ConnectionFailedException before clearing (callers were hanging 10s then getting wrong TimeoutException instead)
  • TaskActionDialogs: fix double-counting in outer catch — subtract already-counted success/skipped from remaining tasks.length
  • TaskDetailsDialog: add outerContext.mounted guard before accessing outerContext.read() in timer callback (prevents crash if underlying page is disposed while timer fires)
  • BuiltinInstanceService._buildArgs: use _resolveEffectiveDhtListenPort for --dht-listen-port instead of raw settings value (ensures valid port range 1-65535 and handles String→int parsing, matching UPnP port resolution)

Summary by CodeRabbit

Bug Fixes

  • Corrected task failure counting to properly account for already-processed tasks and partial progress
  • Added mounted-context checks to prevent operations after dialog disposal
  • WebSocket RPC requests now fail properly instead of hanging when connection closes
  • Improved DHT listen port resolution and validation

Bug fixes:
- Aria2RpcClient.close(): completeError all pending completers with
  ConnectionFailedException before clearing (callers were hanging 10s
  then getting wrong TimeoutException instead)
- TaskActionDialogs: fix double-counting in outer catch — subtract
  already-counted success/skipped from remaining tasks.length
- TaskDetailsDialog: add outerContext.mounted guard before accessing
  outerContext.read<InstanceManager>() in timer callback (prevents
  crash if underlying page is disposed while timer fires)
- BuiltinInstanceService._buildArgs: use _resolveEffectiveDhtListenPort
  for --dht-listen-port instead of raw settings value (ensures valid
  port range 1-65535 and handles String→int parsing, matching UPnP
  port resolution)
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4daa4ed1-3aea-418c-a4eb-a090166001b0

📥 Commits

Reviewing files that changed from the base of the PR and between 0624ca5 and 8a9f331.

📒 Files selected for processing (4)
  • lib/pages/download_page/components/task_action_dialogs.dart
  • lib/pages/download_page/components/task_details_dialog.dart
  • lib/services/aria2_rpc_client.dart
  • lib/services/builtin_instance_service.dart

📝 Walkthrough

Walkthrough

The PR adds four targeted defensive improvements: fixing task failure accounting when exceptions occur mid-batch, guarding async peer-fetch dialogs against disposed contexts, failing pending RPC requests on WebSocket close instead of hanging, and validating DHT port configuration through a resolver function.

Changes

Defensive fixes and safeguards across UI and service layers

Layer / File(s) Summary
Download page dialog error handling and cleanup
lib/pages/download_page/components/task_action_dialogs.dart, lib/pages/download_page/components/task_details_dialog.dart
TaskActionDialogs._performActionForInstance computes additional failures only from uncounted tasks (avoiding double-counting partial progress), and TaskDetailsDialog.showTaskDetailsDialog exits early if the dialog context unmounts before async peer fetching.
Aria2 RPC client lifecycle and DHT configuration
lib/services/aria2_rpc_client.dart, lib/services/builtin_instance_service.dart
WebSocket close() completes all pending RPC completers with ConnectionFailedException instead of abandoning them, and builtin instance startup validates the DHT listen port through _resolveEffectiveDhtListenPort(settings) instead of direct settings fallback.

Possibly related PRs

  • GT-610/setsuna#80: Broader WebSocket connection/close behavior updates in Aria2RpcClient overlap with this PR's pending-request completion logic.
  • GT-610/setsuna#74: Refactors WebSocket notification subsystem and teardown cleanup in the same aria2_rpc_client.dart close-path area.
  • GT-610/setsuna#75: Related Aria2 RPC lifecycle changes including shutdown RPC handling in instance stopping.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix: resolve final 4 bugs before convergence' is vague and generic, using non-descriptive phrasing that does not convey meaningful information about specific bugs being fixed. Replace the generic title with specific bug descriptions, e.g., 'fix: prevent WebSocket RPC timeouts and improve task counting/port validation' to clearly summarize the key fixes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@GT-610 GT-610 merged commit eb71e09 into main Jun 10, 2026
2 checks passed
@GT-610 GT-610 deleted the final-code-fix branch June 10, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant