Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty defaultSettings breaks event reporting #972

Closed
leethree opened this issue Nov 26, 2020 · 2 comments
Closed

Empty defaultSettings breaks event reporting #972

leethree opened this issue Nov 26, 2020 · 2 comments

Comments

@leethree
Copy link

leethree commented Nov 26, 2020

We found there is major data discrepancy between our Android and iOS app using Segment and some iOS clients don't report any data to Segment. After investigation, we found that the library will not report any events if the first call to cdn-settings.segment.com fails.

We are using analytics-react-native and it passes an empty dict for defaultSettings.

https://github.com/segmentio/analytics-react-native/blob/9b5734d58b3f4099b5396504debb4ffad2abdd79/packages/core/src/configuration.ts#L29

This breaks the fallback logic of the library settings because here the code only checks for nil.

} else if (self.configuration.defaultSettings != nil) {
// If settings request fail, load a user-supplied version if present.
// but make sure segment.io is in the integrations
NSMutableDictionary *newSettings = [self.configuration.defaultSettings serializableMutableDeepCopy];
newSettings[@"integrations"][@"Segment.io"][@"apiKey"] = self.configuration.writeKey;
[self setCachedSettings:newSettings];
// don't configure edge functions here. it'll do the right thing on it's own.
} else {

newSettings[@"integrations"][@"Segment.io"][@"apiKey"] doesn't do anything because the keys don't exist. This results in an empty setting cached in the client and no events will be send to Segment.io because the integration is not enabled.

No settings for Segment.io. Skipping.

@bsneed
Copy link
Contributor

bsneed commented Nov 30, 2020

Will have a look into this today.

@bsneed
Copy link
Contributor

bsneed commented Dec 9, 2020

Fixed in PR #973. If any other details come up, please reopen this issue. Thanks so much for the report!!

@bsneed bsneed closed this as completed Dec 9, 2020
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

No branches or pull requests

2 participants