Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase] Fix app exceptions caused by missing App Store receipt #2862

Closed
LHLL wants to merge 14 commits into from
Closed

[in_app_purchase] Fix app exceptions caused by missing App Store receipt #2862

LHLL wants to merge 14 commits into from

Conversation

@LHLL
Copy link
Contributor

@LHLL LHLL commented Jul 6, 2020

Description

Missing App Store receipt can happen for different scenarios, thus shouldn't trigger an exception.

Related Issues

flutter/flutter#43957

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • [ x ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [ x ] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • [ x ] All existing and new tests are passing.
  • [ x ] I updated/added relevant documentation (doc comments with ///).
  • [ x ] The analyzer (flutter analyze) does not report any problems on my PR.
  • [ x ] I read and followed the Flutter Style Guide.
  • [ x ] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • [ x ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • [ x ] I updated CHANGELOG.md to add a description of the change.
  • [ x ] I signed the CLA.
  • [ x ] I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • [ x ] No, this is not a breaking change.

@LHLL LHLL requested review from cyanglaz and mklim as code owners July 6, 2020 23:07
mklim
mklim reviewed Jul 7, 2020
View reviewed changes
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and tests!

packages/in_app_purchase/lib/src/store_kit_wrappers/sk_receipt_manager.dart Outdated
Comment on lines 21 to 23
} catch (_) {
rethrow;
}
Copy link
Contributor

@mklim mklim Jul 7, 2020

Choose a reason for hiding this comment

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

Why the catch and rethrow here? I think this behaves identically to how just returning the await would, since it's rethrowing everything.

Copy link
Contributor Author

@LHLL LHLL Jul 21, 2020

Choose a reason for hiding this comment

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

Reverted.

cyanglaz
cyanglaz reviewed Jul 30, 2020
View reviewed changes
packages/in_app_purchase/CHANGELOG.md
@@ -1,3 +1,6 @@
## 0.3.4+2
Copy link
Contributor

@cyanglaz cyanglaz Jul 30, 2020

Choose a reason for hiding this comment

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

new line after the version

Copy link
Contributor Author

@LHLL LHLL Jul 30, 2020

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@cyanglaz cyanglaz Mar 12, 2021

Choose a reason for hiding this comment

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

The diff seems odd, CHANGELOG probably needs to be re-based

cyanglaz
cyanglaz reviewed Mar 12, 2021
View reviewed changes
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Let's rebase it and land this :) @LHLL

packages/in_app_purchase/CHANGELOG.md
@@ -1,3 +1,6 @@
## 0.3.4+2
Copy link
Contributor

@cyanglaz cyanglaz Mar 12, 2021

Choose a reason for hiding this comment

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

The diff seems odd, CHANGELOG probably needs to be re-based

packages/in_app_purchase/ios/Classes/FIAPReceiptManager.m
NSData *receipt = [self getReceiptData:receiptURL];
NSError *err;
NSData *receipt = [self getReceiptData:receiptURL error:&err];
if (err) {
Copy link
Contributor

@cyanglaz cyanglaz Mar 12, 2021

Choose a reason for hiding this comment

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

Can we also add an xctest to test this?

Copy link
Contributor

@cyanglaz cyanglaz Mar 25, 2021

Choose a reason for hiding this comment

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

friendly ping @LHLL

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 23, 2021

@mvanbeusekom Perhaps you could adopt this PR and get it rebased and xctested?

@renefloor
Copy link
Contributor

renefloor commented Jun 24, 2021 •
edited
Loading

I tried rebasing and cherry picking, but this PR does not go well with our federated architecture changes. I'll just copy paste the changes in a new branch

Edit: @stuartmorgan I added all changes of this PR in 1 commit in #4096, so this outdated PR can be closed if you agree nothing is missing.

renefloor added a commit to Baseflow/flutter-plugins that referenced this pull request Jun 24, 2021
@renefloor @LHLL
Fix app exceptions caused by missing App Store receipt
4536d7d 
Rebase of flutter/pull/2862

Co-authored-by: LHLL <[email protected]>
@renefloor renefloor mentioned this pull request Jun 24, 2021
Merged
1 task
@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 29, 2021

Closing in favor of the new PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@cyanglaz cyanglaz cyanglaz left review comments

@mklim mklim mklim left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes p: in_app_purchase

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@LHLL @stuartmorgan-g @renefloor @cyanglaz @mklim @googlebot