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

fix(background)!: better payment confirmation & message #694

Merged
merged 36 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c9ddb61
fix(background)!: better payment confirmation message
sidvishnoi Nov 1, 2024
05efcb0
wait before first attempt; rename function
sidvishnoi Nov 4, 2024
28d4b03
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 4, 2024
244ab38
make PaymentSession.pay always return OutgoingPayment; retry/throw ot…
sidvishnoi Nov 4, 2024
2148d3f
try different signature: get last outgoingPayment regardless of error
sidvishnoi Nov 4, 2024
a0ee3c6
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 7, 2024
30eb935
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 7, 2024
465ba9c
use async generator in polling; handle more errors; more abstraction
sidvishnoi Nov 7, 2024
951dc74
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 7, 2024
52abeaa
require only outgoingPaymentId in polling
sidvishnoi Nov 7, 2024
c96aa90
nit
sidvishnoi Nov 7, 2024
48cf703
nit: style/import
sidvishnoi Nov 7, 2024
6f93b1c
ui & message improvements
sidvishnoi Nov 8, 2024
d4e89fd
use same error message for `OutgoingPayment.failed = true`
sidvishnoi Nov 8, 2024
d57f918
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 11, 2024
4b13c73
reduce polling initial delay from 2.5s to 1.5s
sidvishnoi Nov 11, 2024
a0768ee
walletAddress check
sidvishnoi Nov 11, 2024
e11cf13
fix typo in error key name
sidvishnoi Nov 11, 2024
3df3acd
comment on null assertion
sidvishnoi Nov 11, 2024
cba0301
use payStatus full/partial instead of success/warn; also in bg
sidvishnoi Nov 11, 2024
d1e3681
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 13, 2024
bca0dbf
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 15, 2024
6560b41
update comment for isMissingGrantPermissionsError
sidvishnoi Nov 15, 2024
387cc08
handle case isMissingGrantPermissionsError is actually isTokenInactiv…
sidvishnoi Nov 15, 2024
715746e
update success/failure states; add partial status; update msgs
sidvishnoi Nov 15, 2024
2979f04
update success message to remove URL
sidvishnoi Nov 15, 2024
34ac427
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 19, 2024
b626ad3
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 22, 2024
3021b41
use opacity animation on message, slide is distracting
sidvishnoi Nov 22, 2024
3c8434c
fix disabled styles
sidvishnoi Nov 22, 2024
d2ae2ce
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 25, 2024
6efb7ba
update _locales key description
sidvishnoi Nov 25, 2024
9032cc4
handle insufficient grant by checking token's access values
sidvishnoi Nov 25, 2024
fb115d1
add todo comment
sidvishnoi Nov 25, 2024
0678549
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 26, 2024
5347047
Merge branch 'main' into payment-confirmation
sidvishnoi Nov 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/background/services/monetization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import type {
} from '@/shared/messages';
import { PaymentSession } from './paymentSession';
import { computeRate, getSender, getTabId } from '../utils';
import {
isMissingGrantPermissionsError,
isOutOfBalanceError,
} from './openPayments';
import { isOutOfBalanceError } from './openPayments';
import {
OUTGOING_PAYMENT_POLLING_MAX_ATTEMPTS,
OUTGOING_PAYMENT_POLLING_MAX_DURATION,
Expand Down Expand Up @@ -329,7 +326,8 @@ export class MonetizationService {
.filter((e) => e.status === 'rejected')
.map((e) => e.reason);

if (pollingErrors.some((e) => isMissingGrantPermissionsError(e))) {
if (pollingErrors.some((e) => e.message === 'InsufficientGrant')) {
this.logger.warn('Insufficient grant to read outgoing payments');
// This permission request to read outgoing payments was added at a
// later time, so existing connected wallets won't have this permission.
// Assume as success for backward compatibility.
Expand Down
28 changes: 16 additions & 12 deletions src/background/services/openPayments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,6 @@ export class OpenPaymentsService {
}: Partial<{ signal: AbortSignal; maxAttempts: number }> = {},
): AsyncGenerator<OutgoingPayment, OutgoingPayment, void> {
let attempt = 0;
let tokenRotated = false;
await sleep(OUTGOING_PAYMENT_POLLING_INITIAL_DELAY);
while (++attempt <= maxAttempts) {
try {
Expand All @@ -917,17 +916,19 @@ export class OpenPaymentsService {
signal?.throwIfAborted();
await sleep(OUTGOING_PAYMENT_POLLING_INTERVAL);
} catch (error) {
if (isMissingGrantPermissionsError(error)) {
if (isTokenInactiveError(error) && !tokenRotated) {
// isMissingGrantPermissionError has same error msg as isTokenInactiveError
tokenRotated = true;
await this.rotateToken();
continue;
if (
isTokenExpiredError(error) ||
isMissingGrantPermissionsError(error)
) {
const token = await this.rotateToken();
const hasReadAccess = token.access_token.access.find(
(e) => e.type === 'outgoing-payment' && e.actions.includes('read'),
);
Comment on lines +926 to +929
Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me. Lets just add a comment, that this specific part can be removed once we have the RS errors in place.

if (!hasReadAccess) {
throw new OpenPaymentsClientError('InsufficientGrant', {
description: error.description,
});
}
throw error;
} else if (isTokenExpiredError(error) && !tokenRotated) {
tokenRotated = true;
await this.rotateToken();
} else {
throw error;
}
Expand Down Expand Up @@ -1022,6 +1023,7 @@ export class OpenPaymentsService {
this.storage.set({ oneTimeGrant: { ...this.grant, accessToken } });
}
this.grant = { ...this.grant, accessToken };
return newToken;
}
}

Expand Down Expand Up @@ -1054,7 +1056,9 @@ export const isSignatureValidationError = (error: any) => {
);
};

export const isTokenExpiredError = (error: any) => {
export const isTokenExpiredError = (
error: any,
): error is OpenPaymentsClientError => {
if (!isOpenPaymentsClientError(error)) return false;
return isTokenInvalidError(error) || isTokenInactiveError(error);
};
Expand Down
3 changes: 0 additions & 3 deletions src/background/services/paymentSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { transformBalance } from '@/popup/lib/utils';
import {
isInvalidReceiverError,
isKeyRevokedError,
isMissingGrantPermissionsError,
isNonPositiveAmountError,
isOutOfBalanceError,
isTokenExpiredError,
Expand Down Expand Up @@ -411,8 +410,6 @@ export class PaymentSession {
if (isKeyRevokedError(e)) {
this.events.emit('open_payments.key_revoked');
throw e;
} else if (isMissingGrantPermissionsError(e)) {
throw e;
} else if (isTokenExpiredError(e)) {
await this.openPaymentsService.rotateToken();
return await this.pay(amount); // retry
Expand Down