DethAriel / ng-recaptcha

Angular component for Google reCAPTCHA
https://dethariel.github.io/ng-recaptcha/
MIT License
469 stars 122 forks source link

ng-recaptcha tag <recaptcha> causes error “zone.js: Unhandled Promise rejection” when route changes #123

Open kovaljames opened 6 years ago

kovaljames commented 6 years ago

[Maintainer update 2022-09-26]

Please see this comment.

Summary

I'm submitting a:

Description

I'm using "ng-recaptcha" on Angular component. Im using inside a form like this:

<form class="form-contact"
        [formGroup]="contactForm"
        #formDirective="ngForm" 
        (ngSubmit)="sendMessage(contactForm, formDirective)">
  <re-captcha class="recaptcha"
              formControlName="captcha"
              siteKey="mysitekey">
  </re-captcha>
</form>

Everything seems to be working fine. However, when route changes (ex. user goes to another page) and the AboutComponent is rendered again, an error pops up (multiple times): Unhandled Promise rejection: timeout ; Zone: ; Task: Promise.then ; Value: timeout undefined. I'm 99% sure it's caused by tag because error doesn't show up after removing the tag. Is there a way to render a captcha without errors when route changes (and captcha reloads after im back to component)? Obs: You must wait a few seconds after navigating to another route and back to captcha, then Promise timeout expires and error shows up inside console (google/firefox)

error

https://stackblitz.com/edit/angular-en3spw

Lib versions:

Mikailus commented 6 years ago

It's not necessary to reload a component to get this error - destroying a component containing recaptcha leading to this case PS. This problem is also present in version 3.0.5 of ng-recaptcha

ErraticFox commented 5 years ago

Does anyone know of a work around with this?

BlueNc commented 5 years ago

I had the same error message, but in my case captcha doesn't load anymore ...

I have 2 forms with captcha, first time the captcha render properly, but when i navigate between them captcha doesn't load anymore.

Only workaround i've found : on form component destroy, remove the captcha element from DOM

  @ViewChild('captchaRef')
  reCaptcha: RecaptchaComponent;

...

  ngOnDestroy() {
    const captchaElem = this.reCaptcha['elementRef'].nativeElement;
    captchaElem.parentElement.removeChild(captchaElem);
  }

Now captcha always load properly, but error message continue to show up...

BrentACole commented 5 years ago

I have the same issue, when the error shows up it breaks some other TS I have running on the next page.

EDIT: I have a guess as to what this is coming from. Seems like the issue is a call to a specific Google URL: https://www.google.com/recaptcha/api2/anchor?[more URL parameters]. Upon navigating away, the library apparently tries to reach this URL, and it just stays at "(Pending)" in dev tools.

DethAriel commented 5 years ago

Hey folks! Thx for letting me know about this, and I’m sorry that getting back to you took so long.

At this point I think I know where the issue is, ~and I plan to release a fix for it in the next couple of days in the 4.x.x branch.~ and I've reached out to recaptcha support team, see my comment below

DethAriel commented 5 years ago

To not keep you folks in the dark, here's a small status update.

Why it happens?

This error happens due to the following scenario:

  1. When the component is destroyed, ng-recaptcha invokes the grecaptcha.reset(ID) method
  2. This results in recaptcha invoking the api2/anchor endpoint that @BrentACole mentions
  3. For some reason this endpoint times out
  4. Since this unhandled promise lies inside recaptcha__en.js file there's no way we could catch it (e.g. patching this file is not an option given such a sophisticated minimization algorithm used)

Why not just skip invoking grecaptcha.reset during ngOnDestroy?

It's there for a reason, and this reason is #10 .

What can I do?

If you can - ignore it. Add this error to an allowlist in your e2e tests or frontend monitoring software.

Otherwise, you can override the ngOnDestroy method of RecaptchaComponent manually (or create a component fork). Please be aware, that this is a temporary and pretty bad fix that might put you in a very unfortunate position when upgrading component versions! Nevertheless, here's approximate code for that:

// somewhere in your "main.ts"
import { RecaptchaComponent } from 'ng-recaptcha';
RecaptchaComponent.prototype.ngOnDestroy = function() {
  if (this.subscription) {
    this.subscription.unsubscribe();
  }
}

What next?

I've submitted this issue to the recaptcha support team, and I'll post an update here when I have it.

Thx for bearing with me on that!

SirWojtek commented 5 years ago

Hey @DethAriel, can you paste the link to the recapcha issue you created?

DethAriel commented 5 years ago

@SirWojtek the recaptcha core team does not have a public bug tracker (at least not one that I know of). I've submitted an issue to their support team through email, and I followed up again today. First time they recommended me to submit a question to StackOverflow, which I did with no results so far.

Let's see if they get back with some hints/workarounds/timelines anytime soon. I would be very happy to bring you the good news today, but right now this is all I've got

ErraticFox commented 5 years ago

I had someone Ina discord give me something like this which works most of the time but sometimes will still throw errors.


@ViewChild('captcha') captcha: ElementRef;
@ViewChild('submit') submit: MatButton;

captchaConnection;

renderCaptcha() {
    if (!grecaptcha.render) {
        setTimeout(() => {
            grecaptcha.render(this.captcha.nativeElement, {
                callback: (e) => this.captchaCallback(e)
            });
        }, 2000)
    }
    grecaptcha.render(this.captcha.nativeElement, {
        callback: (e) => this.captchaCallback(e)
    });
}
DethAriel commented 5 years ago

@ErraticFox it does not look like this is solving the issue, could you please elaborate on what you mean by "works most of the time"? These errors are thrown when recaptcha is removed from DOM, not when it's rendered

LuiisFernando commented 5 years ago

Anyone solved this error ? I had the same error on angular 4 with recaptcha.

h2software commented 5 years ago

Trying @DethAriel's workaround on Angular 7, I'm getting:

ERROR in src/main.ts(23,12): error TS2339: Property 'subscription' does not exist on type 'RecaptchaComponent'.

@DethAriel also writes

this is a temporary and pretty bad fix that might put you in a very unfortunate position when upgrading component versions

I am not sure what this means? If it is pretty bad, does it not actually fix the bug?

fxck commented 5 years ago

hm, I tried overiding the destroy event as suggested in https://github.com/DethAriel/ng-recaptcha/issues/123#issuecomment-426112101

and doesn't seems to have any effect

matudelatower commented 5 years ago

now with the @DethAriel https://github.com/DethAriel/ng-recaptcha/issues/123#issuecomment-426112101

throws: image

DethAriel commented 5 years ago

@matudelatower seems to still work fine for me (see this example), but as I mentioned, this is not a fix, and my current suggestion is to ignore this error and add it to the allow-list in the frontend monitoring tools.

haisham commented 4 years ago

If someone is facing this issue in ionic/cordova app, i solved it by setting in config.xml, this would also need cordova whitelist plugin

LSzelecsenyi commented 4 years ago

Has anybody found a solution to avoid this?

Cobr3n commented 4 years ago

Hello anybody found a solution, now ? maybe we can load script outside angular ? or catch errors manual ?

corpulent commented 4 years ago

I am still testing, but this seemed to work for me.

import { FormControl, Validators } from '@angular/forms';
import { RecaptchaComponent } from 'ng-recaptcha';
public captchaEl: FormControl = new FormControl(null, Validators.required);
  ngOnInit(): void {
    RecaptchaComponent.prototype.ngOnDestroy = function() {
      if (this.subscription) {
        this.subscription.unsubscribe();
      }
    }

    this.captchaEl.reset();
  }

Then after the form is submitted, I do this.captchaEl.reset(); again. Still testing, but I am not seeing the error anymore.

ErraticFox commented 4 years ago

@DethAriel any other updates or responses from the reCaptcha team?

PRR24 commented 4 years ago

Google sadly has a bad habit of not providing cleanup functions for their APIs. reset() is semantically wrong method anyway, there should be a destroy() call.

I have played around a lot with simplified version of the project, having static grecaptcha api loaded in index.html and a tiny component with grecaptcha.render in ngAfterViewInit and different cleanup experiments in ngOnDestroy. I have failed to get it working reliably. Creating and destroying the component multiple times either leaks memory, results an error or both.

RonRofe commented 4 years ago

To not keep you folks in the dark, here's a small status update.

Why it happens?

This error happens due to the following scenario:

  1. When the component is destroyed, ng-recaptcha invokes the grecaptcha.reset(ID) method
  2. This results in recaptcha invoking the api2/anchor endpoint that @BrentACole mentions
  3. For some reason this endpoint times out
  4. Since this unhandled promise lies inside recaptcha__en.js file there's no way we could catch it (e.g. patching this file is not an option given such a sophisticated minimization algorithm used)

Why not just skip invoking grecaptcha.reset during ngOnDestroy?

It's there for a reason, and this reason is #10 .

What can I do?

If you can - ignore it. Add a whitelist for this error in your e2e tests or frontend monitoring software.

Otherwise, you can override the ngOnDestroy method of RecaptchaComponent manually (or create a component fork). Please be aware, that this is a temporary and pretty bad fix that might put you in a very unfortunate position when upgrading component versions! Nevertheless, here's approximate code for that:

// somewhere in your "main.ts"
import { RecaptchaComponent } from 'ng-recaptcha';
RecaptchaComponent.prototype.ngOnDestroy = function() {
  if (this.subscription) {
    this.subscription.unsubscribe();
  }
}

What next?

I've submitted this issue to the recaptcha support team, and I'll post an update here when I have it.

Thx for bearing with me on that!

Hi! Any news from google

johngrimsey commented 4 years ago

Hey @DethAriel have you tried moving the offending logic outside a zone?

constructor(private ngZone: NgZone) {
}

...

this.ngZone.runOutsideAngular(() => {
   // reCAPTCHA logic here
})

https://angular.io/api/core/NgZone#runoutsideangular

Just a thought!

PRR24 commented 4 years ago

@johngrimsey No, it does not help.

johngrimsey commented 4 years ago

Ah well. Still, great lib man.

ErraticFox commented 4 years ago

Could this be of some help? I'm on mobile so it's difficult to check, but thought I'd share so someone else could try this. Worth a shot. https://github.com/google/recaptcha/issues/269#issuecomment-606838861

gsinghkular commented 4 years ago

anyone found a fix for it? I'm still seeing this in version 5

mprojs commented 3 years ago

As a temporary solution, I use a RouteReuseStrategy. This allows you not to destroy the page (route) where the captcha is used, but to save it to the cache. As a result, the ngOnDestroy method for this component is never called and no error is thrown. The downside of this solution is that the component remains in memory, but in my case it is a small page with a feedback form, so I decided simply ignore it.

nicholasconfer commented 3 years ago

Just adding to this bug. I'm getting the same error using the latest version of ng-recaptcha, but the issue now points to zone.evergreen.js. The original solution provided using main.ts also seems to have no effect. Going to keep working on this issue, but just wanted others to know it can also appear as zone.evergreen.js

image

Cassidie commented 3 years ago

Just adding to this bug. I'm getting the same error using the latest version of ng-recaptcha, but the issue now points to zone.evergreen.js. The original solution provided using main.ts also seems to have no effect. Going to keep working on this issue, but just wanted others to know it can also appear as zone.evergreen.js

image

I am also experiencing this! :(

nicholasconfer commented 3 years ago

Wanted to add a couple notes to the group. I spent a good deal of time working on this problem, looking at alternative libraries on github, and going so far as to even write my own component to take a fresh perspective. Long story short, I think the ng-recaptcha library is good.

A couple notes...

  1. This error is unavoidable from best I can tell, and similar to the authors original notes it appears to be on Google's end. They don't have a dispose option, which makes things very difficult.

  2. You can override the dispose method using RecaptchaComponent.prototype.ngOnDestroy as mentioned in the original answer by the author, but I would strongly discourage this at this point. When going this route, you will remove the cancel error caused when calling google's recaptcha service and switching routes mid-stream, and you will also not get an unhandled promise rejection error. However, working on the latest version of Angular, I noticed after a longer period of time I would get Timeout errors with this method of solution, and these would persist. The reason the other errors go away is you are no longer destroying the iframes created by Google's recaptcha. In other words you create a new more persistent error, and you make the DOM busier. This method may have worked in the past, but I wouldn't recommend it today.

I've got an improved workaround solution to the problem I think, but still need to do some final testing. I'll post an update here shortly in the next day or two with an update.

gyomi95 commented 3 years ago

Hi all!

Last week, I also ran into this problem myself. I found that destroying, then re-instantiating the ng-recaptcha component between route changes may be the cause of this bug, as the newly instantiated component is not able to handle the promise when the recaptcha expires, and the resolved(response: string) method is being called back. The workaround I found for this, is that I added the ng-recaptcha component to the root component of my application like so:

app.component.html:

<router-outlet></router-outlet>
<re-captcha
  #captchaRef="reCaptcha"
  (resolved)="resolved($event)"
  size="invisible">
</re-captcha>

Then I implemented a service for offering the token as an observable to components in which I'm using recaptcha, so they can subscibe to the event:

captchav2.service.ts:

private recaptchav2 = new Subject<string>();
recaptchav2$ = this.recaptchav2.asObservable();
constructor() { }
recaptchav2Resolved(token: string){
  this.recaptchav2.next(token);
}

To wire the service with the actual callback method:

app.component.ts

public resolved(captchaResponse: string): void {
  this.captchav2Service.recaptchav2Resolved(captchaResponse);
}

Then in the component, in which I want to use recaptcha (in my case, it is a component for registration):

register.component.ts:

constructor( ...  private recaptchav2Service: Recaptchav2Service) {
}
private subscription: Subscription;

ngOnInit(): void {
  this.subscription = this.recaptchav2Service.recaptchav2$.subscribe((token) => {
    this.sendRegisterForm(token)
  })
...
}

onRegisterBtnClick(): void {
  grecaptcha.execute();
}

sendRegisterForm(token: string) {
  // do what you need to do here
...
  grecaptcha.reset();
  this.http.post(...).subscribe(() => {
    // or here :)
  });
}

public ngOnDestroy() {
  if(this.subscription){
    this.subscription.unsubscribe()
  }
}

So my solution works by having a single instance of the ng-recaptcha component throughout the entire application. In my case, it's good enough. This solution probably won't work properly if you need to have multiple instances of the ng-recaptcha component for whatever reason.

I advise you to have the badge hidden by default:

.grecaptcha-badge {
  visibility: hidden;
}

Then, you can either un-hide the badge on the pages you want to use it (this can be tricky!), or just include the text suggested by google on the page:

This site is protected by reCAPTCHA and the Google
    <a href="https://policies.google.com/privacy">Privacy Policy</a> and
    <a href="https://policies.google.com/terms">Terms of Service</a> apply.

Note, that I'm not legally responsibe for the validity of this information, so you better check out Google's the terms on this matter

What do you think of that? If you see any downsides of this solution, please let me know.

nicholasconfer commented 3 years ago

The answer posted by @gyomi95 is what I was planning to do as well, got busy last week on other projects. Basically the issue is not easy to fix until Google deals with it, so the best path is to create it at the root so its not destroyed and you can reuse it across components.

PRR24 commented 3 years ago

While I hope the above suggestion helps someone, I would like to point out few disadvantages to consider while using it:

Leks12lk commented 3 years ago

@DethAriel are there any news regarding this issue? It happens also for ng-recaptcha v 5.0.0.

Odonno commented 3 years ago

@DethAriel Your solution seems to work (latest version on a angular 10 project).

I know it's a temporary solution but it's still better than nothing!

fabichacon commented 2 years ago

Anyone solved this error ?

brennomarques commented 1 year ago

I'm having the same problem, using v2 of recaptcha version 8.0.1 and an error always appears.

image

I'm using ngOnDestroy but it still doesn't work.

Can anyone help me? thanks.

ammad172 commented 1 year ago

This issue still exists in "ng-recaptcha": "^10.0.0" is there any fix available ??

fegyi001 commented 1 year ago

This issue still exists in "11.0.0".....

estebanlazza commented 1 year ago

I'm having the same problem, using v2 of recaptcha version 8.0.1 and an error always appears.

image

I'm using ngOnDestroy but it still doesn't work.

Can anyone help me? thanks.

I was having this error because I forgot add the domain on Google reCAPTCHA settings.

sumyunggun commented 1 year ago

Hey @corpulent I'm unsure where your subscription here comes from, could you please elaborate as I would also like to try and implement this fix


  ngOnInit(): void {
    RecaptchaComponent.prototype.ngOnDestroy = function() {
      if (this.subscription) {
        this.subscription.unsubscribe();
      }

    this.captchaEl.reset();
  }

EDIT: For clarification, I am using reCaptchaV2 with the checkbox. I just realised that your implementation could be for the invisible type

amigo740 commented 1 year ago

In search of a solution to the error, I noticed that by adding the following entry to the component, the error disappears. I'm using Angular v12 and ng-recaptcha v8

import { RecaptchaComponent } from 'ng-recaptcha';

ngOnInit() {
    RecaptchaComponent.prototype.ngOnDestroy = function() {};
}
DionysusDT commented 1 year ago

Is this problem serious and needs to be fixed or can I ignore it? Thank you

sumyunggun commented 12 months ago

I'm no longer using this implementation so you can ignore if you'd like, but I'm sure I'm not the only one with the same issue.

nicholasconfer commented 12 months ago

I would vote for a fix if possible.

dannielnavas commented 10 months ago

In version 15 of Angular, the following worked for me: RecaptchaComponent.prototype.ngOnDestroy = function () {};. I added this to the main.ts file.