[google_sign_in] Fixes PlataformException leaking in debug mode (#37343)

[feinstein]

Description

Prevents PlatformException from leaking in debug mode, as Dart is not able to support an exception thrown in a future because the event loop that handles the future is not able to see the caller's stack trace.

Related Issues

flutter/flutter#37343
flutter/flutter#26705
dart-lang/sdk#37268

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.

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

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[cyanglaz]

Thank you for the contribution! The code looks good!
Do you mind add a test for this. It should be easily testable.

[feinstein]

Thank you for the contribution! The code looks good!
Do you mind add a test for this. It should be easily testable.

I was thinking about how to add a test for this and I am not sure how to make it, since the PlatformException only leaks in debug mode and when it leaks all try-catch statements can't catch it, how can I simulate this? How can I simulate debug mode and how can I see if an Exception leaked if I can't catch it?

[cyanglaz]

I was thinking about how to add a test for this and I am not sure how to make it, since the PlatformException only leaks in debug mode and when it leaks all try-catch statements can't catch it, how can I simulate this? How can I simulate debug mode and how can I see if an Exception leaked if I can't catch it?

I haven't tried, but I think you can test it by creating a mock platform class to handle the method channel. Some examples can be found here:

plugins/packages/in_app_purchase/test/in_app_purchase_connection/app_store_connection_test.dart

Lines 272 to 436 in 374a35a

	class FakeIOSPlatform {
	FakeIOSPlatform() {
	channel.setMockMethodCallHandler(onMethodCall);
	}
	// pre-configured store informations
	String receiptData;
	Set<String> validProductIDs;
	Map<String, SKProductWrapper> validProducts;
	List<SKPaymentTransactionWrapper> transactions;
	List<SKPaymentTransactionWrapper> finishedTransactions;
	bool testRestoredTransactionsNull;
	bool testTransactionFail;
	PlatformException queryProductException;
	PlatformException restoreException;
	SKError testRestoredError;
	void reset() {
	transactions = [];
	receiptData = 'dummy base64data';
	validProductIDs = ['123', '456'].toSet();
	validProducts = Map();
	for (String validID in validProductIDs) {
	Map productWrapperMap = buildProductMap(dummyProductWrapper);
	productWrapperMap['productIdentifier'] = validID;
	validProducts[validID] = SKProductWrapper.fromJson(productWrapperMap);
	}
	SKPaymentTransactionWrapper tran1 = SKPaymentTransactionWrapper(
	transactionIdentifier: '123',
	payment: dummyPayment,
	originalTransaction: dummyTransaction,
	transactionTimeStamp: 123123123.022,
	transactionState: SKPaymentTransactionStateWrapper.restored,
	error: null,
	);
	SKPaymentTransactionWrapper tran2 = SKPaymentTransactionWrapper(
	transactionIdentifier: '1234',
	payment: dummyPayment,
	originalTransaction: dummyTransaction,
	transactionTimeStamp: 123123123.022,
	transactionState: SKPaymentTransactionStateWrapper.restored,
	error: null,
	);
	transactions.addAll([tran1, tran2]);
	finishedTransactions = [];
	testRestoredTransactionsNull = false;
	testTransactionFail = false;
	queryProductException = null;
	restoreException = null;
	testRestoredError = null;
	}
	SKPaymentTransactionWrapper createPendingTransactionWithProductID(String id) {
	return SKPaymentTransactionWrapper(
	payment: SKPaymentWrapper(productIdentifier: id),
	transactionState: SKPaymentTransactionStateWrapper.purchasing,
	transactionTimeStamp: 123123.121,
	transactionIdentifier: id,
	error: null,
	originalTransaction: null);
	}
	SKPaymentTransactionWrapper createPurchasedTransactionWithProductID(
	String id) {
	return SKPaymentTransactionWrapper(
	payment: SKPaymentWrapper(productIdentifier: id),
	transactionState: SKPaymentTransactionStateWrapper.purchased,
	transactionTimeStamp: 123123.121,
	transactionIdentifier: id,
	error: null,
	originalTransaction: null);
	}
	SKPaymentTransactionWrapper createFailedTransactionWithProductID(String id) {
	return SKPaymentTransactionWrapper(
	payment: SKPaymentWrapper(productIdentifier: id),
	transactionState: SKPaymentTransactionStateWrapper.failed,
	transactionTimeStamp: 123123.121,
	transactionIdentifier: id,
	error: SKError(
	code: 0,
	domain: 'ios_domain',
	userInfo: {'message': 'an error message'}),
	originalTransaction: null);
	}
	Future<dynamic> onMethodCall(MethodCall call) {
	switch (call.method) {
	case '-[SKPaymentQueue canMakePayments:]':
	return Future<bool>.value(true);
	case '-[InAppPurchasePlugin startProductRequest:result:]':
	if (queryProductException != null) {
	throw queryProductException;
	}
	List<String> productIDS =
	List.castFrom<dynamic, String>(call.arguments);
	assert(productIDS is List<String>, 'invalid argument type');
	List<String> invalidFound = [];
	List<SKProductWrapper> products = [];
	for (String productID in productIDS) {
	if (!validProductIDs.contains(productID)) {
	invalidFound.add(productID);
	} else {
	products.add(validProducts[productID]);
	}
	}
	SkProductResponseWrapper response = SkProductResponseWrapper(
	products: products, invalidProductIdentifiers: invalidFound);
	return Future<Map<String, dynamic>>.value(
	buildProductResponseMap(response));
	case '-[InAppPurchasePlugin restoreTransactions:result:]':
	if (restoreException != null) {
	throw restoreException;
	}
	if (testRestoredError != null) {
	AppStoreConnection.observer
	.restoreCompletedTransactionsFailed(error: testRestoredError);
	return Future<void>.sync(() {});
	}
	if (!testRestoredTransactionsNull) {
	AppStoreConnection.observer
	.updatedTransactions(transactions: transactions);
	}
	AppStoreConnection.observer
	.paymentQueueRestoreCompletedTransactionsFinished();
	return Future<void>.sync(() {});
	case '-[InAppPurchasePlugin retrieveReceiptData:result:]':
	if (receiptData != null) {
	return Future<void>.value(receiptData);
	} else {
	throw PlatformException(code: 'no_receipt_data');
	}
	break;
	case '-[InAppPurchasePlugin refreshReceipt:result:]':
	receiptData = 'refreshed receipt data';
	return Future<void>.sync(() {});
	case '-[InAppPurchasePlugin addPayment:result:]':
	String id = call.arguments['productIdentifier'];
	SKPaymentTransactionWrapper transaction =
	createPendingTransactionWithProductID(id);
	AppStoreConnection.observer
	.updatedTransactions(transactions: [transaction]);
	sleep(const Duration(milliseconds: 30));
	if (testTransactionFail) {
	SKPaymentTransactionWrapper transaction_failed =
	createFailedTransactionWithProductID(id);
	AppStoreConnection.observer
	.updatedTransactions(transactions: [transaction_failed]);
	} else {
	SKPaymentTransactionWrapper transaction_finished =
	createPurchasedTransactionWithProductID(id);
	AppStoreConnection.observer
	.updatedTransactions(transactions: [transaction_finished]);
	}
	break;
	case '-[InAppPurchasePlugin finishTransaction:result:]':
	finishedTransactions
	.add(createPurchasedTransactionWithProductID(call.arguments));
	break;
	}
	return Future<void>.sync(() {});
	}
	}

And in your mock platform class, just throw an exception.

Now in your test case, you can do something like

bool exceptionCaught;
try {
  googleSignIn.signIn();
} catch(e) {
  exceptionCaught = true;
}
expect(exceptionCaught, true);

Without your change, exceptionCaught should never be assigned to true.

[feinstein]

The google_sign_in_test.dart has some setUp() that are already handling the method channel, can't I just use something like them?

[cyanglaz]

@feinstein Yes, if it helps. Also feel free to edit the existing handler or create another if it doesn't suffice your need.

[feinstein]

There's a test already there which might be the same code as you suggested:

plugins/packages/google_sign_in/test/google_sign_in_test.dart

Lines 268 to 273 in 374a35a

	test('signInSilently suppresses errors by default', () async {
	channel.setMockMethodCallHandler((MethodCall methodCall) {
	throw "I am an error";
	});
	expect(await googleSignIn.signInSilently(), isNull); // should not throw
	});

Wouldn't this test do the same? It failed to catch this problem.

[cyanglaz]

there is a suppressError parameter in signInSilently() that you might need to set to false to make it throw.

[feinstein]

It was throwing before I fixed it, the example app starts with a signInSilently(), which immidiately throws into the IDE, when I fixed it, it stopped throwing. It makes sense, as the VM is "ignoring" all catch blocks, and signInSilently() ignores the exceptions by catching them.

The whole issue was that the VM was ignoring try-catch blocks, so the configuration inside signInSilently() shouldn't matter.

[cyanglaz]

It was throwing before I fixed it, the example app starts with a signInSilently(), which immidiately throws into the IDE, when I fixed it, it stopped throwing. It makes sense, as the VM is "ignoring" all catch blocks, and signInSilently() ignores the exceptions by catching them.

The whole issue was that the VM was ignoring try-catch blocks, so the configuration inside signInSilently() shouldn't matter.

Makes sense, I think we can merge this now since the test is covered.

[feinstein]

I think this fix was regressed as I am experiencing the same problem after an update.

[maheshj01]

@feinstein the google sign in cancelled issue still persissts flutter/flutter#17677 (comment)

[feinstein]

I know, and I communicated this on other issues but no one seemed to care.

[maheshj01]

it would be really helpful for the community if someone could really fix it.

[feinstein]

I don't think it will be an easy fix, the whole code was changed to a more generic one, which makes ir really difficult to fix, as the fix is to remove then() for await.

[pr-Mais]

I have this same issue, isn't there a workaround for the moment?

[feinstein]

Compile in Release, this only happens in Debug.

[ahumaro]

I can confirm this issue is still present today. I am using version 4.1.1

[maheshj01]

I can confirm this issue is still present today. I am using version 4.1.1

I can confirm it works well in release mode

[brianschardt]

i can confirm this is still an issue in debug mode