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

#onGiveUp #75

Open
rvbabilonia opened this issue Jul 10, 2023 · 4 comments
Open

#onGiveUp #75

rvbabilonia opened this issue Jul 10, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@rvbabilonia
Copy link

rvbabilonia commented Jul 10, 2023

Hey guys,

Can #onGiveUp throw a different exception? For example, the #execute will keep running while hitting a pending exception but when max attempts have been breached, the #onGiveUp will invoke another API call before throwing a different exception. Am I missing something?

public awaitSuccessfulQuickPayment(quickPaymentId: string, maxWaitSeconds: number): Promise<QuickPaymentResponse> {
         const retryPolicy = retry(handleType(BlinkPendingException), {
             maxAttempts: maxWaitSeconds,
             backoff: new ConstantBackoff(1000)
         });
         retryPolicy.onGiveUp(async data => {
             const blinkConsentTimeoutException = new BlinkConsentTimeoutException();
             try {
                 await this.revokeQuickPaymentAsync(quickPaymentId);
                 this._logger.info(
                     "The max wait time was reached while waiting for the quick payment to complete and the payment has been revoked with the server. Quick payment ID: ",
                     quickPaymentId);
             } catch (revokeException) {
                 if (revokeException != undefined) {
                     this._logger.error(
                         "Waiting for the quick payment was not successful and it was also not able to be revoked with the server due to: ",
                         revokeException.message, quickPaymentId);
                     throw new AggregateError([blinkConsentTimeoutException, revokeException]);
                 }
             }
             throw blinkConsentTimeoutException;
         });
         return retryPolicy
             .execute(async () => {
                 const quickPayment = await this.getQuickPaymentAsync(quickPaymentId);
                 const status = quickPayment.data.consent.status;
                 this._logger.debug("The last status polled was: " + status + " \tfor Quick Payment ID: " + quickPaymentId);
                 if (status === ConsentStatusEnum.Authorised) {
                     return quickPayment;
                 }
                 throw new BlinkPendingException();
             })
             .then(response => response.data)
             .catch(error => {
                 if (error instanceof BlinkConsentFailureException || error instanceof BlinkPendingException || error instanceof BlinkServiceException) {
                     throw error;
                 } else if (error && error.innerException) {
                     if (error.innerException instanceof BlinkServiceException) {
                         throw error.innerException;
                     }
                     throw new BlinkServiceException(error.innerException.message);
                 } else {
                     throw new BlinkServiceException(error.message, error);
                 }
             });
 }
@connor4312
Copy link
Owner

I'm not clear on what you're asking for here. What is the benefit of "a different exception", where would it come from?

@rvbabilonia
Copy link
Author

  1. The policy is handling BlinkPendingException (retries are working).
  2. When the retries have been exhausted, I want the function to return BlinkConsentTimeoutException.
  3. There's also a possibility that on give up, the revocation API call also throws an exception that needs to be appended into consent timeout exception.

Here's the Polly counterpart I wrote for C# .NET:

      var retryPolicy = Policy<QuickPaymentResponse>
           .Handle<BlinkConsentFailureException>()
           .WaitAndRetryAsync(maxWaitSeconds, retryAttempt => TimeSpan.FromSeconds(1),
               (exception, timeSpan, retryCount, context) =>
               {
                   if (retryCount == maxWaitSeconds)
                   {
                       var blinkConsentTimeoutException = new BlinkConsentTimeoutException();

                       try
                       {
                           RevokeQuickPaymentAsync(quickPaymentId).Wait();
                           _logger.LogInformation(
                               "The max wait time was reached while waiting for the quick payment to complete and the payment has been revoked with the server. Quick payment ID: {consentId}",
                               quickPaymentId);
                       }
                       catch (Exception revokeException)
                       {
                           _logger.LogError(
                               "Waiting for the quick payment was not successful and it was also not able to be revoked with the server due to: {message}. Quick payment ID: {consentId}",
                               revokeException.Message, quickPaymentId);
                           throw new AggregateException(blinkConsentTimeoutException, revokeException);
                       }

                       throw blinkConsentTimeoutException;
                   }
               });

@connor4312
Copy link
Owner

I see. Currenly this library doesn't have anything analogous to that, and onGiveUp is just a 'notification', not a handler. This should not be hard to implement by hand in your code, but I see the utility in having a dedicated handler.

@connor4312 connor4312 added the enhancement New feature or request label Jul 11, 2023
@rvbabilonia
Copy link
Author

Thanks, mate.

My workaround is to handle the BlinkPendingException in the catch block and do the revocation from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants