Setting the apiHost in the configuration has no effect #156

jgable · 2022-09-20T16:59:50Z

Describe the bug
Setting the apiHost in the configuration has no effect, events are still sent to api.segment.io/v1 regardless of the configuration values for apiHost.

let configuration = Configuration(writeKey: writeKey)
   .trackApplicationLifecycleEvents(true)
   .apiHost("analytics-ingestion.kraftful.com")
   .flushInterval(10)

Analytics.debugLogsEnabled = true
analytics = Analytics(configuration: configuration)

Steps for repro:

I have created an example app with repro in a fork: https://github.com/kraftful/analytics-swift/tree/jgable/api-host-repro (specific commit is ce5331d)

Clone the above branch/commit
Load the ExampleApp project in XCode (in the Examples/APIHostConfigurationRepro folder)
Put breakpoints in the HTTPClient file in Segment package where the batch uploads happen
Run the app on simulator (I have tried iPhone 12 Pro)
Login using creds: [email protected], test1234
Observe the value of uploadURL in the debugger when the batch is uploaded

Expected:

Should be analytics-ingestion.kraftful.com since that is passed in the configuration in the SegmentAnalytlics.swift file

Actual:

api.segment.io/v1 is used instead.

Expected behavior

Event batches should be sent to the configured apiHost.

Screenshots

Platform (please complete the following information):

Library Version in use: latest
Platform being tested: iOS (latest)
Integrations in use: none

The text was updated successfully, but these errors were encountered:

bsneed · 2022-10-18T00:39:09Z

This currently works as intended. The idea was before you've received results from Segment's /settings endpoint, you may have a preference for EU vs US endpoints. Segment settings has the final say, but we acknowledge this is problematic when intending to use a proxy (which I assume you're wanting to do). We'll look at this soon and try and figure out a good path forward. In the meantime feel free to fork and comment out the following line in SegmentDestination.swift

apiHost = segmentInfo?[Self.Constants.apiHost.rawValue] as? String

That'll make it such that whatever you supply will be used at all times.

jackgene · 2022-10-28T12:39:01Z

This currently works as intended. The idea was before you've received results from Segment's /settings endpoint, you may have a preference for EU vs US endpoints. Segment settings has the final say, but we acknowledge this is problematic when intending to use a proxy (which I assume you're wanting to do). We'll look at this soon and try and figure out a good path forward. In the meantime feel free to fork and comment out the following line in SegmentDestination.swift
apiHost = segmentInfo?[Self.Constants.apiHost.rawValue] as? String
That'll make it such that whatever you supply will be used at all times.

If this is "working as intended", can you please documented it clearly in the Analytics Swift documentation - https://segment.com/docs/connections/sources/catalog/libraries/mobile/swift-ios/

I just spent several hours last night trying to understand why our events aren't going through our proxy with the apiHost configuration set.

In fact, from my troubleshooting, it is unclear to me if what you are saying is accurate (that apiHost is being used as a default). Browsing through the source code, I do not see the apiHost value on the configuration object ever being read.

Anyway, I've opened this ticket https://segment.zendesk.com/hc/requests/488619

jackgene · 2022-10-28T17:04:14Z

Just to confirm, this is the workaround you are proposing, correct?
main...jackgene:analytics-swift:main

If so, it is not working for me. As I pointed out in my last comment, apiHost on the Configuration object doesn't appear to be being read at all. Let me know if I've missed anything else.

Thank you.

bsneed · 2022-10-28T17:34:34Z

@jackgene It is being read, I assure you. See HTTPClient.swift. That workaround is no longer necessary once this PR is merged: #165. Documentation will be updated before a point release is made.

jackgene · 2022-10-28T18:26:18Z

Appreciate your assurance, but the Swift debugger disagrees with you. But if you can point me to the actual line of code where the apiHost from the configuration object is being passed to HttpClient, that will facilitate our troubleshooting. Alternatively, please give us a timeline on when your proposed fix will be merged. I want to mention that as a paying customer, we do not appreciate your arrogant tone. Thank you. Regards, Jack

…

On Oct 28, 2022, at 10:34 AM, Brandon Sneed ***@***.***> wrote: @jackgene It is being read, I assure you. See HTTPClient.swift. That workaround is no longer necessary once this PR is merged: #165. Documentation will be updated before a point release is made. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

bsneed · 2022-10-28T18:33:40Z

@jackgene There was no "tone". I'm just sharing facts. As an employee here trying to help you, I don't appreciate yours so lets take it down a notch. See the line below as to where it's being read.

analytics-swift/Sources/Segment/Utilities/HTTPClient.swift

Lines 30 to 52 in 93314b0

    
           init(analytics: Analytics, apiKey: String? = nil, apiHost: String? = nil, cdnHost: String? = nil) { 
        
               self.analytics = analytics 
        
               if let apiKey = apiKey { 
        
                   self.apiKey = apiKey 
        
               } else { 
        
                   self.apiKey = analytics.configuration.values.writeKey 
        
               } 
        
               if let apiHost = apiHost { 
        
                   self.apiHost = apiHost 
        
               } else { 
        
                   self.apiHost = Self.defaultAPIHost 
        
               } 
        
               if let cdnHost = cdnHost { 
        
                   self.cdnHost = cdnHost 
        
               } else { 
        
                   self.cdnHost = Self.defaultCDNHost 
        
               } 
        
               self.session = Self.configuredSession(for: self.apiKey) 
        
           }

The fix has already been merged since we first spoke. A point release has yet to happen, pending documentation.

jackgene · 2022-10-28T20:29:18Z

@jackgene There was no "tone". I'm just sharing facts. As an employee here trying to help you, I don't appreciate yours so lets take it down a notch. See the line below as to where it's being read.

analytics-swift/Sources/Segment/Utilities/HTTPClient.swift

Lines 30 to 52 in 93314b0

init(analytics: Analytics, apiKey: String? = nil, apiHost: String? = nil, cdnHost: String? = nil) {

self.analytics = analytics

if let apiKey = apiKey {

self.apiKey = apiKey

} else {

self.apiKey = analytics.configuration.values.writeKey

}

if let apiHost = apiHost {

self.apiHost = apiHost

} else {

self.apiHost = Self.defaultAPIHost

}

if let cdnHost = cdnHost {

self.cdnHost = cdnHost

} else {

self.cdnHost = Self.defaultCDNHost

}

self.session = Self.configuredSession(for: self.apiKey)

}

The fix has already been merged since we first spoke. A point release has yet to happen, pending documentation.

The lines you highlighted is the initializer for HTTPClient (called "constructor" in other languages). It shows that the initializer accepts an apiHost, and defaults it to nil when no value is passed in - more on initializers here - https://docs.swift.org/swift-book/LanguageGuide/Initialization.html. But the key point is these lines of code do not show apiHost being read from the Configuration object.

(Note that in Swift (and in most programming languages really), the fact that there's a constructor parameter named apiHost, and that there's a corresponding property named apiHost on the Configuration object, does not automatically bind the two)

For Configuration's apiHost to be passed to the initializer, it has to be explicitly invoked with it, it would look something like this:

let configuration = Configuration("some write key").apiHost("myhost.com/v1")
let analytics = Analytics(configuration: configuration)
HTTPClient(
  analytics: analytics,
  apiHost: configuration.values.apiHost // THIS LINE
)

If you look through the analytics-swift source, you'll find that the HTTPClient is being initialized from three different places (you can find this in Xcode by right clicking on the init function -> Find -> Find Call Hierarchy):

You'll note that in none of these locations is apiHost being set to a value read from Configuration.

Another point worth noting, you "assured me" that Configuration.Values.apiHost is being read. Here's a tip on how you can quickly see that that is not the case:

Navigate to the apiHost property for Configuration.Values (https://github.com/segmentio/analytics-swift/blob/main/Sources/Segment/Configuration.swift#L34) in Xcode.
Right-click on apiHost, and then Find -> Find Call Hierarchy

You'll quickly see it's not referenced anywhere. Try the same thing with say writeKey, and you'll see all the places where writeKey is being read from.

bsneed · 2022-10-28T20:55:46Z

Thanks for all those super useful links.

Here, I'll paste the code for you since it seems difficult for you to see it on GitHub.

    init(analytics: Analytics, apiKey: String? = nil, apiHost: String? = nil, cdnHost: String? = nil) {
        self.analytics = analytics
        
        if let apiKey = apiKey {
            self.apiKey = apiKey
        } else {
            self.apiKey = analytics.configuration.values.writeKey
        }

Low and behold, apiKey gets set from the configuration value that was specified if null was passed into the constructor. HTTPClient is a somewhat generic class that lost its way a little over time. But wait, there's more!

Just a few lines down ....

    @discardableResult
    func startBatchUpload(writeKey: String, batch: URL, completion: @escaping (_ result: Result<Bool, Error>) -> Void) -> URLSessionDataTask? {
        guard let uploadURL = segmentURL(for: apiHost, path: "/b") else {
            completion(.failure(HTTPClientErrors.failedToOpenBatch))
            return nil
        }

Enjoy.

jackgene · 2022-10-28T21:08:36Z

You may want to re-read this issue.

It is about apiHost not applying to the HTTPClient, NOT apiKey or writeKey.

bsneed · 2022-10-28T22:55:16Z

Ya got me. My apologies, apiKey has been front and center for me all week. I'll make a PR for the default setting of apiHost in the coming days, however that doesn't seem like what you needed in the first place. The linked PR has more along the lines of what you and the original poster of this bug were looking for. See Configuration.requestFactory which gives you full control over all the details of a given request (headers/url/path/etc).

bsneed · 2022-10-28T23:04:04Z

Actually, I'll reopen this as another issue that omits the confusion.

jackgene · 2022-10-28T23:35:55Z

Ya got me. My apologies, apiKey has been front and center for me all week. I'll make a PR for the default setting of apiHost in the coming days, however that doesn't seem like what you needed in the first place. The linked PR has more along the lines of what you and the original poster of this bug were looking for. See Configuration.requestFactory which gives you full control over all the details of a given request (headers/url/path/etc).

I think you are right, that the requestFactory override is more what I need.

And apologies if my frustration was coming through, but my point was that I knew I was writing the apiHost value on the Configuration object, but I wasn't seeing it have any effect.

Going through the code manually, and having Xcode try and find all references to Configuration.Values.apiHost, I find a couple of references where the value is written to, but no references where it is ever read from, hence I don't see how it could possibly be affecting any behavior of HTTPClient or anything else.

Thanks for opening the new issue, and have a nice weekend!

bsneed · 2022-10-29T16:29:46Z

No worries at all! Glad requestFactory does the trick for you and you're back in business. Hope you have a good weekend as well. Ttys!

jgable added the triage label Sep 20, 2022

jgable assigned bsneed Sep 20, 2022

bsneed mentioned this issue Oct 25, 2022

feat: add requestFactory to Configuration #165

Merged

bsneed closed this as completed Oct 28, 2022

bsneed reopened this Oct 28, 2022

bsneed mentioned this issue Oct 28, 2022

Setting of apiHost is not being honored before being overwritten by /settings endpoint. #167

Closed

bsneed closed this as completed Oct 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Setting the apiHost in the configuration has no effect #156

Setting the apiHost in the configuration has no effect #156

jgable commented Sep 20, 2022

bsneed commented Oct 18, 2022

Uh oh!

jackgene commented Oct 28, 2022 •

edited

Loading

Uh oh!

jackgene commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 via email

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 •

edited

Loading

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 •

edited

Loading

Uh oh!

bsneed commented Oct 29, 2022

Uh oh!

Setting the apiHost in the configuration has no effect #156

Setting the apiHost in the configuration has no effect #156

Comments

jgable commented Sep 20, 2022

bsneed commented Oct 18, 2022

Uh oh!

jackgene commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jackgene commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 via email

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

bsneed commented Oct 28, 2022

Uh oh!

jackgene commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bsneed commented Oct 29, 2022

Uh oh!

jackgene commented Oct 28, 2022 •

edited

Loading

jackgene commented Oct 28, 2022 •

edited

Loading

jackgene commented Oct 28, 2022 •

edited

Loading