Skip to content

Removed Retry Logic from BNCServerRequestOperation Class - To keep Be…#1582

Merged
NidhiDixit09 merged 1 commit into4.0.0-alpha.0from
Alpha-Bug-Fixes
Feb 25, 2026
Merged

Removed Retry Logic from BNCServerRequestOperation Class - To keep Be…#1582
NidhiDixit09 merged 1 commit into4.0.0-alpha.0from
Alpha-Bug-Fixes

Conversation

@NidhiDixit09
Copy link
Copy Markdown
Collaborator

…havior similar to Android

Reference

SDK-XXXX -- <TITLE>.

Summary

Motivation

Type Of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing Instructions

cc @BranchMetrics/saas-sdk-devs for visibility.

@matterai-app
Copy link
Copy Markdown
Contributor

matterai-app Bot commented Feb 24, 2026

Code Quality type: bug fix

Summary By MatterAI MatterAI logo

🔄 What Changed

This PR removes the internal retry logic from the BNCServerRequestOperation class in the Branch iOS SDK. It simplifies the request execution flow by directly processing the server response and finishing the operation without attempting multiple retries within this specific class. It also ensures that response processing and callback execution for BranchEventRequest occur synchronously on the main thread.

🔍 Impact of the Change

The removal of internal retries shifts the responsibility of request persistence or retry logic elsewhere in the SDK or to the caller. This reduces complexity and potential side effects within the operation queue but may lead to immediate failures for transient network issues if not handled at a higher level. The use of BNCPerformBlockOnMainThreadSync ensures UI-safe callback execution.

📁 Total Files Changed

Click to Expand
File ChangeLog
Simplify Request BNCServerRequestOperation.m Removed retry logic, simplified callback handling, and enforced main thread execution for response processing.

🧪 Test Added/Recommended

Recommended

  • Unit Test: Verify that a failed network request immediately triggers the completion handler without retrying.
  • Integration Test: Ensure BranchEventRequest callbacks are correctly executed on the main thread.
  • Regression Test: Confirm that the removal of retries does not break the BranchOpenRequest sequence.

🔒 Security Vulnerabilities

No direct security vulnerabilities detected. The change focuses on logic simplification and threading.

⏳ Estimated code review effort

LOW (~8 minutes)

Tip

Quality Recommendations

  1. Verify if retry logic is now handled by BNCServerInterface or a higher-level manager to prevent regression in flaky network conditions.

  2. Ensure BNCPerformBlockOnMainThreadSync handles potential deadlocks if the current thread is already the main thread.

  3. Add verbose logging for the specific error reason when a request fails since retries are no longer attempted.

♫ Tanka Poem

Loops of retry fade, 📉
Logic flows in simple lines, 🌊
Main thread holds the hand, 🧵
Data finds its final home, 🏠
Clean code brings us peace. 🧪

Sequence Diagram

sequenceDiagram
    participant Op as BNCServerRequestOperation
    participant Req as BNCServerRequest
    participant SI as BNCServerInterface
    participant CB as BNCCallbackMap

    Op->>Op: executeRequest()
    Op->>Req: makeRequest(serverInterface, branchKey, callback)
    Req->>SI: perform network call
    SI-->>Req: BNCServerResponse, NSError
    Req-->>Op: callback(response, error)

    Note over Op: Main Thread Sync Block
    Op->>Req: processResponse(response, error)
    
    opt is BranchEventRequest
        Op->>CB: callCompletionForRequest(request, success, error)
    end

    Op->>Op: finishOperation()
Loading

@matterai-app
Copy link
Copy Markdown
Contributor

matterai-app Bot commented Feb 24, 2026

✅ Reviewed the changes: Identified a high-priority threading issue that could lead to deadlocks and main thread contention.

@NidhiDixit09 NidhiDixit09 merged commit 77f4e33 into 4.0.0-alpha.0 Feb 25, 2026
10 checks passed
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.

2 participants