Skip to content

Conversation

NidhiDixit09
Copy link
Collaborator

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.

Copy link
Contributor

matter-code-review bot commented Sep 23, 2025

Code Quality new feature

Summary By MatterAI MatterAI logo

🔄 What Changed

This Pull Request enhances the request tracing mechanism within the Branch SDK. It introduces new properties (requestParams and requestServiceURL) to BranchInstallRequest and BranchOpenRequest to store request details. Crucially, the callbackForTracingRequests typedef has been updated to include the requestServiceURL as a new parameter, which is then passed to the traceCallback in BranchOpenRequest. 🚀

🔍 Impact of the Change

These changes significantly improve the observability and debugging capabilities for network requests made by the SDK. By exposing the service URL to custom tracing callbacks, developers gain deeper insights into the request lifecycle, facilitating better monitoring and troubleshooting. ✨

📁 Total Files Changed

  • Sources/BranchSDK/BranchInstallRequest.m: Now stores request parameters and the service URL as properties before making the network call.
  • Sources/BranchSDK/BranchOpenRequest.m: Stores the service URL and updates the traceCallback invocation to include this new URL.
  • Sources/BranchSDK/Private/BranchOpenRequest.h: Declares requestParams and requestServiceURL as public properties for BranchOpenRequest.
  • Sources/BranchSDK/Public/BNCCallbacks.h: Modifies the callbackForTracingRequests signature to accept the requestServiceURL.

🧪 Test Added

No explicit tests were added in this Pull Request. 🚫

🔒Security Vulnerabilities

No new security vulnerabilities were introduced or fixed by these changes. 🛡️

Tip

Quality Recommendations

  1. Add unit tests for the traceCallback in BranchOpenRequest to ensure the requestServiceURL parameter is correctly passed and received.

  2. Verify proper memory management for requestParams and requestServiceURL properties to prevent potential leaks, especially with copy and readwrite attributes.

Tanka Poem ♫

New data flows, a URL,
Traced now with keen, watchful eye.
Callbacks enriched, insights bloom,
Code's journey, clearer, high.
Observability's sweet gain. 🔬✨

Sequence Diagram

sequenceDiagram
    participant RI as BranchInstallRequest
    participant RO as BranchOpenRequest
    participant RF as BNCRequestFactory
    participant BSA as BNCServerAPI
    participant BSI as BNCServerInterface
    participant TC as traceCallback

    Note over RI,RO: Prepare Request
    RI->>RF: dataForInstallWithURLString(urlString)
    RF-->>RI: params
    RI->>BSA: installServiceURL()
    BSA-->>RI: installURL
    RI->>RI: self.requestParams = params
    RI->>RI: self.requestServiceURL = installURL
    RI->>BSI: postRequest(params, installURL, key, callback)

    RO->>RF: dataForOpenWithURLString(urlString)
    RF-->>RO: params
    RO->>BSA: openServiceURL()
    BSA-->>RO: openURL
    RO->>RO: self.requestParams = params
    RO->>RO: self.requestServiceURL = openURL
    RO->>BSI: postRequest(params, openURL, key, callback)

    Note over RO: Process Response
    BSI-->>RO: response (async)
    RO->>TC: traceCallback(urlString, requestParams, response.data, error, requestServiceURL)
Loading

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use MatterAI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with MatterAI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use MatterAI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with MatterAI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use MatterAI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with MatterAI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use MatterAI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with MatterAI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

};

[invocation setArgument:&_odmFetchCompletion atIndex:3];
[invocation retainArguments];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed because ODM info goes out of scope and garbage collected?

@NidhiDixit09 NidhiDixit09 merged commit 8d8f5ce into master Sep 24, 2025
22 of 23 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