Conversation
anagstef
requested review from
agis,
chanioxaris,
dimkl,
nikosdouvlis and
panteliselef
🦋 Changeset detectedLatest commit: e30bde7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| console.error('Clerk: Failed to load the CAPTCHA script from the URL: ', url); | ||
| throw { | ||
| captchaError: 'captcha_script_failed_to_load', | ||
| }; |
There was a problem hiding this comment.
🙃 I would suggest we throw a ClerkError (an existing one or create a new one) instead of throwing an object. In order to provide a better error handling in the future i think that we will need to use our Error classes.
There was a problem hiding this comment.
It's a bit complicated! We don't want everything to throw here. We actually send the captchaError error value to the FAPI.
| } catch (_) { | ||
| } catch { | ||
| // Rethrow with specific message | ||
| clerkFailedToLoadThirdPartyScript('Cloudflare Turnstile'); |
There was a problem hiding this comment.
🙃 Why did we drop the usage of clerkFailedToLoadThirdPartyScript ? Couldn't we enhance it with an optional url parameter?
There was a problem hiding this comment.
As described above, the clerkFailedToLoadThirdPartyScript throws an Error object which is not what we want to do here.
|
I think it's worth noting that, even more important that the console log that this PR achieves, is the request that we'll make to FAPI's signUp.create in case there's an error loading the script - this was not the case before this PR. I'm mentioning this for reviewers, because it might not be apparent how this is achieved by looking at the code changes. |
…ose-more-information-on-turnstile
anagstef
deleted the
stefanos/core-2247-investigate-if-we-can-expose-more-information-on-turnstile
branch
* fix(clerk-js): Improve logging for CAPTCHA script loading errors * fix(clerk-js): Remove the generic CAPTCHA console error (cherry picked from commit 34befee)
…lure logging (#3384) * fix(clerk-js): Improve logging for CAPTCHA script loading errors (#3374) * fix(clerk-js): Improve logging for CAPTCHA script loading errors * fix(clerk-js): Remove the generic CAPTCHA console error (cherry picked from commit 34befee) * fix(clerk-js): Use first-party cookies when running on Cypress (#3245) * fix(clerk-js): Use first-party cookies when running on Cypress * fix(clerk-js): Create isCypress util (cherry picked from commit 7b213d5)
5 participants
Description
This PR adds a
console.erroron every CAPTCHA error.Also, when script loading fails, we log it in the FAPI, meaning that the
/sign_upsAPI call will be executed and will include the following value:Checklist
npm testruns as expected.npm run buildruns as expected.Type of change