Stuck in bad state when passing improper phone number format

walkingbrad commented 7 months ago

Description

If we call set(profile:) with a profile containing a phone number that the API doesn't like (e.g. empty string, though presumably other values as well), the SDK seems to get stuck in a bad state that prevents profile syncing from working, even if we later set the phone number to nil in code. I notice warning logs that show a 400 http error from the Klaviyo API indicating that the phone number is invalid. This persists even after I then attempt to set the phone number to nil.

Besides handling phone number as an empty string, I also worry that this same behavior would occur if the phone number we pass belongs to a country that Klaviyo does not yet have SMS support for. Some conversations I found online from Klaviyo staff indicated that this may be the case.

For what it's worth, I don't believe the Android SDK has the same issue. I pass an empty phone number string there and I think all is well.

My app sets the profile in a reactive manner whenever the "customer stream" emits a new customer value. This occurs immediately on app start, on sign in / register, on profile information update, and on sign out (in which we call resetProfile()).

I assume nil values are interpreted as "ignore", where the internal state management retains previously set non-nil values. So it seems impossible to effectively remove a previously-set field.

It may be worth considering switching from a raw String to an enum that allows more granularity. I've seen other tools implement something similar. Example:

enum ProfileValue {
  case unset
  case set(String?)
}

Checklist

[X] I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
[X] If possible, I've reproduced the issue using the main branch of this package.
[X] This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

Either:

an empty phone number string is parsed out by the SDK and not sent up to the API
setting a profile field to nil wipes out the previous field value (though I understand this has other implications, so probs not a solution)

Actual behavior

The Klaviyo SDK holds onto old profile values (the phone number empty string) and does not unset them when we later try to set it to nil, resulting in all future API calls to fail.

Steps to reproduce

Set profile phone number to empty string
Notice API call fail with an invalid phone number error
Set profile phone number to nil
Notice API call continue to fail for the same initial reason

The Klaviyo Swift SDK version information

2.2.1

Destination operating system

iOS 17

Xcode version information

15

Swift Compiler version information

No response

ndurell commented 7 months ago

Thanks for the report we'll take a closer look at it this morning!

ajaysubra commented 6 months ago

@walkingbrad we were able to reproduce this on our end and are working on fixing it. Will mark this issue evergreen and report back when we have a fix. Thanks again for reporting this issue.

ajaysubra commented 6 months ago

@walkingbrad this is now fixed here and should be available in the next release. We are working on the details of when we'll release but should be in the near future. Thanks again for reporting and I'll close this issue now.

ajaysubra commented 6 months ago

@walkingbrad release 2.4.0 has this fix.

klaviyo / klaviyo-swift-sdk