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

Notify on checkout err #428

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

laur89
Copy link

@laur89 laur89 commented Feb 5, 2025

Note this depends on #427

- note NotifierService.sendNotification() signature already declared
  'url' param as optional, but this change fixes some of the implementations
  that didn't respect the optionality.
@claabs
Copy link
Owner

claabs commented Feb 5, 2025

This can work if we're okay with there being no message on the notification.

I'd also like to change the reason from PURCHASE_ERROR to something like RUNTIME_ERROR. PURCHASE_ERROR is a legacy enum from when the project could actually make purchases. Where these errors are caught, it could be for various reasons.

@laur89 laur89 force-pushed the notify-on-checkout-err branch from c24025b to 697c3a6 Compare February 5, 2025 01:38
@laur89
Copy link
Author

laur89 commented Feb 5, 2025

Personally I'd be okay with no message. Reason alone is enough to just pull my attention to check out what's happening, I don't need stacktraces on my phone.

I'd also like to change the reason

Dunno, IMHO it states pretty accurately what's going on - something went haywire at purchase/checkout step.

@laur89 laur89 force-pushed the notify-on-checkout-err branch 2 times, most recently from 51ff055 to 9ea146b Compare February 6, 2025 14:42
@laur89 laur89 force-pushed the notify-on-checkout-err branch from 9ea146b to abe0c6b Compare February 6, 2025 14:46
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

Successfully merging this pull request may close these issues.

2 participants