Skip to content

Conversation

LekoArts
Copy link
Contributor

@LekoArts LekoArts commented Jun 5, 2024

Description

So inside our sign-in verification machine we use assertIsDefined a bunch. Those errors weren't displayed anywhere, you could only see them if you've used NEXT_PUBLIC_CLERK_ELEMENTS_DEBUG=true.

In SDK-1470 I recorded how I got into such a state some time ago. Actually, I wasn't able to reproduce now because our conditional logic is better now. However, if for some reason a user gets into the state of assertIsDefined throwing, they'll now see it at least 👍

Fixes SDK-1470

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Copy link

changeset-bot bot commented Jun 5, 2024

🦋 Changeset detected

Latest commit: b446576

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clerk/elements Patch

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

@LekoArts LekoArts changed the title fix(elements): Display assertIsDefined errors during development fix(elements): Display assertIsDefined errors during development Jun 5, 2024
if (process.env.NODE_ENV === 'development') {
assertActorEventError(event);

throw new ClerkElementsRuntimeError(`Unable to fulfill the prepare or attempt request for the sign-in verification.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can say prepare/attempt here because the action is only used in those two states

}

assertIsDefined(params);
assertIsDefined(params, 'First factor params');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a label to all instances of assertIsDefined. If params would be undefined it would say:

undefined is not defined

With the label it'll say

First factor params is not defined

👍

@LekoArts LekoArts marked this pull request as ready for review June 5, 2024 09:56
Copy link
Member

@panteliselef panteliselef left a comment

Choose a reason for hiding this comment

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

lgtm

@LekoArts LekoArts enabled auto-merge (squash) June 5, 2024 10:40
@LekoArts LekoArts merged commit 46a0a7b into main Jun 5, 2024
@LekoArts LekoArts deleted the lekoarts/sdk-1470-handle-assertisdefined-better branch June 5, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants