Fix Quick Match cancel button not responding

[Rozerxshashank]

🐛 Bug

The Cancel button in Quick Match could be tapped before the pairing request
was fully created, leading to a null-assertion crash and causing the UI
to get stuck on the loading screen.

✅ Fix

• Guarded against null requestDocId
• Safely closed realtime subscriptions
• Ensured navigation always occurs via a finally block

🧪 Testing

• Verified logic-level fix for the race condition
• Cancel action now always exits pairing screen safely
• Google Sign-In could not be tested locally due to Appwrite OAuth configuration

Fixes #670

Summary by CodeRabbit

• Bug Fixes
  • Improved request cancellation reliability with enhanced error handling and recovery mechanisms.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

🎉 Welcome @Rozerxshashank!
Thank you for your pull request! Our team will review it soon. 🔍

• Please ensure your PR follows the contribution guidelines. ✅
• All automated tests should pass before merging. 🔄
• If this PR fixes an issue, link it in the description. 🔗

We appreciate your contribution! 🚀

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The cancelRequest method in the Pair Chat controller has been refactored to add comprehensive error handling. It now wraps deletion and subscription closure logic in a try/catch/finally structure with conditional checks, ensuring the cancel operation completes even when errors occur.

Changes

Cohort / File(s)	Summary
Error Handling Improvements in Cancel Flow lib/controllers/pair_chat_controller.dart	Refactored cancelRequest method with try/catch/finally structure. Now conditionally deletes the request document only if requestDocId is non-null, resets the ID, closes subscriptions with error logging, and guarantees navigation to tab view via finally block for improved robustness.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

• Verify the conditional deletion logic correctly handles null requestDocId scenarios
• Confirm subscription closure order doesn't cause resource leaks or unintended side effects
• Ensure the finally block guarantee of navigation doesn't suppress critical error states

Poem

🐰 A button that failed to obey,
Now catches its faults along the way,
With try-catch-finally standing strong,
The cancel flow rights what was wrong,
Quick Match loading won't delay! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix Quick Match cancel button not responding' directly and clearly summarizes the main change: addressing the unresponsive Cancel button in Quick Match.
Linked Issues check	✅ Passed	The changes implement all coding requirements from issue #670: guarding against null requestDocId, safely closing subscriptions, and ensuring navigation via finally block for reliable cancel operation.
Out of Scope Changes check	✅ Passed	All modifications to cancelRequest() are directly scoped to fixing the race condition and null-assertion crash described in issue #670.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Rozerxshashank]

Hi @M4dhav 👋
I’ve opened a PR that fixes this issue.
Looking forward to your review. Thanks!

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/controllers/pair_chat_controller.dart (1)

241-260: Excellent fix for the race condition!

The try/catch/finally structure with null-guarded deletion properly addresses the bug described in issue #670. The finally block ensures navigation always occurs, preventing the stuck UI, and the null check prevents the crash when cancel is tapped before the document is created.

Optional: Move requestDocId = null to the finally block for guaranteed cleanup.

Currently, if document deletion fails (lines 244-248), the exception is caught but requestDocId remains non-null until navigation completes. While Get.offNamedUntil should dispose the controller, moving the reset to finally would be more defensive:

🔎 View suggested refactor

 Future<void> cancelRequest() async {
   try {
     if (requestDocId != null) {
       await databases.deleteDocument(
         databaseId: masterDatabaseId,
         collectionId: pairRequestCollectionId,
         documentId: requestDocId!,
       );
     }
-
-    requestDocId = null;
-
     await subscription?.close();
     await userAddedSubscription?.close();
   } catch (e) {
     log('Cancel request failed: $e');
   } finally {
+    requestDocId = null;
     Get.offNamedUntil(AppRoutes.tabview, (route) => false);
   }
 }

Optional: Consider independent error handling for subscriptions.

If subscription?.close() throws on line 253, userAddedSubscription?.close() won't execute. Wrapping each in separate try/catch would ensure both closures are attempted:

🔎 View alternative approach

Future<void> cancelRequest() async {
  try {
    if (requestDocId != null) {
      await databases.deleteDocument(
        databaseId: masterDatabaseId,
        collectionId: pairRequestCollectionId,
        documentId: requestDocId!,
      );
    }
  } catch (e) {
    log('Failed to delete request document: $e');
  }

  try {
    await subscription?.close();
  } catch (e) {
    log('Failed to close subscription: $e');
  }

  try {
    await userAddedSubscription?.close();
  } catch (e) {
    log('Failed to close userAddedSubscription: $e');
  }

  requestDocId = null;
  Get.offNamedUntil(AppRoutes.tabview, (route) => false);
}

This provides more granular error reporting and guarantees all cleanup attempts execute.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b063ecb and 6e94b21.

📒 Files selected for processing (1)

• lib/controllers/pair_chat_controller.dart (1 hunks)

[M4dhav]

Please fix merge conflicts

[M4dhav]

For future contributions, please strictly follow the PR Template

[Rozerxshashank]

Resolved the merge conflict and restored the safe cancelRequest implementation.
The cancel flow now guards against null requestDocId and always navigates back safely.

[Rozerxshashank]

Hi @M4dhav 👋
Just a gentle follow-up on this PR.
I've resolved the merge conflicts and addressed the feedback.
Please let me know if anything else is needed from my side.
Thanks!

[M4dhav]

Hey @Rozerxshashank , automated tests are failing

[M4dhav]

fix tests failure

[Rozerxshashank]

Hey @M4dhav, I’ve pushed the updated fix for PairChatController.cancelRequest.
Please let me know if any further changes are needed from my side.