Skip to content

feat: better auth#5701

Open
rebelchris wants to merge 16 commits intomainfrom
feat-better-auth
Open

feat: better auth#5701
rebelchris wants to merge 16 commits intomainfrom
feat-better-auth

Conversation

@rebelchris
Copy link
Contributor

@rebelchris rebelchris commented Mar 11, 2026

Changes

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://feat-better-auth.preview.app.daily.dev

@vercel
Copy link

vercel bot commented Mar 11, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Mar 11, 2026 1:15pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
storybook Ignored Ignored Mar 11, 2026 1:15pm

Request Review

@rebelchris
Copy link
Contributor Author

@claude make sure all circleCI is happy and green

@claude
Copy link
Contributor

claude bot commented Mar 11, 2026

Claude finished @rebelchris's task in 35m 36s —— View job


Checking CircleCI Status

  • Check CI status for the PR
  • Identify failing/problematic tests
  • Fix useRegistration.ts - implement auto sign-in after sign-up
  • Add @marsidev/react-turnstile mock in jest config
  • Fix RegistrationForm.tsx - display hints['traits.email'] error
  • Fix RegistrationForm.spec.tsx - BetterAuth test setup issues
  • Fix AccountSecurityPage.tsx - BetterAuth test waitForNock issue

Issues Found

packages/shared tests (RegistrationForm.spec.tsx)

  1. renderBetterAuthRegistration calls waitForNock() but Kratos interceptors (set up by renderComponent()) are never consumed when isBetterAuth=true — causes timeout
  2. Form submission blocked by @marsidev/react-turnstile returning undefined in JSDOM (not mocked)
  3. Experience level dropdown not selected → form validation fails early
  4. Second email check (from RegistrationForm.handleFormSubmit) needs a nock mock
  5. Missing feature: auto sign-in after sign-up in useRegistration.ts (the BETTER_AUTH_SIGNUP_FALLBACK_ERROR constant is defined but never used)
  6. hints['traits.email'] not rendered in RegistrationForm, so error message never appears in DOM

packages/webapp tests (AccountSecurityPage.tsx)
7. Better Auth test calls waitForNock() after renderComponent() which sets up Kratos interceptors never consumed with isBetterAuth=true

Comment on lines +38 to +40
} catch {
return { error: fallbackError } as BetterAuthResult<T>;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add some logging with logEvent error for catch in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Claude says:
⏺ Looking at the file, the only catch block (line 38) handles JSON parse failures on error responses. A few considerations:

  1. This is a plain utility file, not a React component — it doesn't have access to useLogContext(), so you'd need to either pass a logger in or import a non-React logging mechanism.
  2. The catch is narrow and expected — it fires when an error response has a non-JSON body. The caller already gets the fallbackError string, so it's not a silent failure.
  3. It's a fetch-level concern — API errors are typically better logged at the call site (the component/hook) where you have context about what the user was doing.

Adding logging here would add coupling for minimal benefit. If you're seeing issues where API errors go unnoticed, a better approach would be logging at the call sites where you have access to LogContext and
can include meaningful context (e.g., which action failed, user state).

That said, if you want lightweight visibility without React context, a console.warn in the catch would be low-cost:

} catch (e) {
console.warn('Failed to parse error response:', path, e);
return { error: fallbackError } as BetterAuthResult;
}

But I'd lean toward no change — the current behavior is fine for a startup moving fast. Want me to add anything?

Copy link
Contributor

Choose a reason for hiding this comment

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

was just thinking to be aware of issues happening, maybe we can log these on server side since it goes through API?

@rebelchris rebelchris requested a review from capJavert March 11, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants