-
Notifications
You must be signed in to change notification settings - Fork 316
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
Fixed purchasing with PromotionalOffer
s using StoreKit 2
#2020
Conversation
f40b7bb
to
a2d3047
Compare
@@ -122,11 +122,20 @@ extension PromotionalOffer.SignedData { | |||
|
|||
@available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *) | |||
var sk2PurchaseOption: Product.PurchaseOption { | |||
let signature: Data | |||
|
|||
if let decoded = Data(base64Encoded: self.signature) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping this would fix it, but the test is still failing :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm out of ideas, I'll probably have to file a Radar and pray? 🙏🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like one of our customers tried to do this without RC, using StoreKit
directly, and he gets the same result, so that confirms the fact that this is an Apple bug.
He's opened a TSI with Apple, so hopefully they can address this soon.
In the mean time, I'll leave this PR open. I believe the fix here will still be required (doing the base64
decode). But once Apple fixes it on their end we can use this PR to confirm, as well as the now enabled test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have the ticket # with Apple?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the original, but we reported it today as well under: FB11764024.
if let decoded = Data(base64Encoded: self.signature) { | ||
signature = decoded | ||
} else { | ||
// TODO: log warning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely worth logging something loud here.
Stuff logged from here would also show up in the diagnostics API call, right? since we get offerings and parse products?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely worth logging something loud here.
👍🏻
Stuff logged from here would also show up in the diagnostics API call, right? since we get offerings and parse products?
This property is only used when purchasing though.
…ons` This changes the default back to `StoreKit 1`. We decided to do this for the following reasons: - Purchasing with `PromotionalOffer`s does not work with StoreKit 2 due to an Apple bug (see #2020 (comment)) - `checkTrialOrIntroDiscountEligibility` is significantly slower with StoreKit 2 (#1893). We're adding optimizations to help with that (#2007), but the underlying logic will still be slow. - A rare race-condition where `StoreKit 2` does not have transactions after a purchase ([TRIAGE-82]). We have some workarounds (#1945), but it's still being investigated. _ Note: This effectively reverts 0ee540a. That commit made it easier to only change the default in one place which is why this PR is basically just one line._
…ons` (#2022) This changes the default back to `StoreKit 1`. We decided to do this for the following reasons: - Purchasing with `PromotionalOffer`s does not work with StoreKit 2 due to an Apple bug (see #2020 (comment)) - `checkTrialOrIntroDiscountEligibility` is significantly slower with `StoreKit 2` (#1893). We're adding optimizations to help with that (#2007), but the underlying logic will still be slow. - A rare race-condition where `StoreKit 2` does not have transactions after a purchase ([TRIAGE-82]). We have some workarounds (#1945), but it's still being investigated. _Note: This effectively reverts 0ee540a. That commit made it easier to only change the default in one place which is why this PR is basically just one line._ [TRIAGE-82]: https://revenuecats.atlassian.net/browse/TRIAGE-82?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
I just realized this cursed PR is # 2020 🙈 |
#destiny |
a2d3047
to
58e9c22
Compare
58e9c22
to
2c82f1f
Compare
Update: We noticed today that this started to work actually. Also, I can confirm that the decoding logic in this PR is important. When not decoding the signature, I was getting the Something I'm a bit confused about is the fact that this is now working even when using a simulator and a StoreKit testing store file. I thought that in that scenario, Xcode won't make any remote request, so I cannot understand how this was fixed from the App Store server side. Glad it is though. |
Interesting, our integration test is still failing. I'll dig more next week though. |
Weird, I can't get this to work even with this fix. Are you doing something else differently? Is it running on iOS 16.1.2 only or something? |
2c82f1f
to
22beb89
Compare
PromotionalOffer
sStoreKit 2
: fix purchasing with PromotionalOffer
s
My personal device is still at 15.7. I could get it working on it, plus when using Xcode 14.1 with a simulator and a StoreKit test store file. Another colleague confirmed he could get it working on 15.7 as well. Not sure this is relevant to you, but I noticed that when using the sample server app Apple attached here to generate the signature, you shouldn't omit the |
The test cases we validated included when you're an active member or a former member. Meaning, purchasing a subscription with a promo offer when you have an active subscription already, and purchasing a subscription with a promo offer after cancelling a previous subscription. |
Thanks, I just ran it on iOS 15.x and it's still failing. I verified in our backend that the username is never empty. |
I love how the docs aren't even updated for SK2? |
By never empty, do you mean that you're actually using a value when generating the signature? If that's the case, then you'll have to pass an |
Whoa we're definitely not doing that. This is still not working: options.insert(signedData.sk2PurchaseOption)
options.insert(.appAccountToken(UUID(uuidString: self.appUserID)!)) But I feel like we're on the right track. I missed this:
|
(Testing now calling |
Our backend isn't lowercasing the user ID for SK2, so that's likely the issue. |
CONFIRMED! Thanks a lot for that pointer @mamouneyya! We'll get this fixed. |
Wohoo! Glad you got it working! 🎉 |
This will be used for https://github.com/RevenueCat/khepri/pull/4843, to fix [SDKONCALL-160] and #2020.
22beb89
to
5038cef
Compare
StoreKit 2
: fix purchasing with PromotionalOffer
sPromotionalOffer
s using StoreKit 2
if let decoded = Data(base64Encoded: self.signature) { | ||
signature = decoded | ||
} else { | ||
throw Error.failedToDecodeSignature(self.signature) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I improved error handling here (the last TODO) and added a few tests.
@@ -376,8 +376,6 @@ class StoreKit1IntegrationTests: BaseBackendIntegrationTests { | |||
@available(iOS 15.2, tvOS 15.2, macOS 12.1, watchOS 8.3, *) | |||
func testPurchaseWithPromotionalOffer() async throws { | |||
try AvailabilityChecks.iOS15APIAvailableOrSkipTest() | |||
try XCTSkipIf(Self.storeKit2Setting == .enabledForCompatibleDevices, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
thinking more about this: |
Here's an example base64 encoded signature:
And decoded:
I suppose if the backend sent it without encoding, and we create the |
This will be used for https://github.com/RevenueCat/khepri/pull/4843, to fix [SDKONCALL-160] and #2020.
5038cef
to
4f4e0f8
Compare
…d` (#2118) This will be used for https://github.com/RevenueCat/khepri/pull/4843, to fix [SDKONCALL-160] and #2020. `X-StoreKit2-Setting` only told us if the apps had that setting enabled. However, when running on an older device, `SK2` would still be disabled. [SDKONCALL-160]: https://revenuecats.atlassian.net/browse/SDKONCALL-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
4f4e0f8
to
24b938e
Compare
// This should be base64 | ||
signature: "signature \(Int.random(in: 0..<1000))", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not base64 the result of this so it looks realistic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is
testPurchaseSK2PackageWithInvalidPromotionalOfferSignatureThrowsError
So I'm testing that if it's not base64 (so it's just a random string), it produces the correct error instead of crashing or something.
Fixes [SDKONCALL-160].
24b938e
to
a36c1b6
Compare
Merging 🎉 |
This solves my problem, Im just passing empty string in |
Fixes SDKONCALL-160, SDKONCALL-182, #2114, and RevenueCat/react-native-purchases#455.
Depends on #2118 and https://github.com/RevenueCat/khepri/pull/4843 / https://github.com/RevenueCat/khepri/pull/4852.
Changes:
signature