Skip to content

fix(cross-domain): don't log a spurious error on callbacks without newSession - #417

Open
mishafyi wants to merge 1 commit into
convex:mainfrom
mishafyi:fix/cross-domain-link-social-redirect
Open

mishafyi wants to merge 1 commit into
convex:mainfrom
mishafyi:fix/cross-domain-link-social-redirect

Conversation

@mishafyi

Copy link
Copy Markdown

Supersedes #389 — that PR got auto-closed when I force-pushed this branch to a bad (parentless) commit by mistake; GitHub won't reopen a PR after a force-push to unrelated history. This is the same branch, fixed: one commit, one file. Original review thread with @erquhart is on #389.

Problem

The cross-domain after-hook fires on every OAuth / magic-link callback and mints a one-time token from ctx.context.newSession for the cross-domain cookie handoff. When the user is already authenticated — e.g. linkSocial — no new session is created, so newSession is undefined and the hook hits:

if (!session) {
  ctx.context.logger.error("No session found");
  return;
}

…logging an error on a completely normal path.

Change

Drop the logger.error on that branch (and document why it's expected). As @erquhart noted on #389, the early return doesn't swallow anything: the provider's location redirect already set on the response passes through unchanged, so the browser still returns to the app — only the OTT handoff is (correctly) skipped when there's no new session. No behavioral change.

Note

This started as a redirect fix we carried as a prod patch on the 0.10.x line. We've since removed the crossDomain plugin from our own setup, so I can't re-verify the original dead-end against current better-auth — going with @erquhart's read that the redirect is preserved on the no-session path, and narrowing this to just the misleading log. Existing src/plugins/cross-domain/index.test.ts still passes (6/6).

…wSession

The cross-domain after-hook runs on every OAuth/magic-link callback and
mints a one-time token from ctx.context.newSession for the cross-domain
cookie handoff. On callbacks where the user is already authenticated —
e.g. linkSocial — no new session is created, so newSession is undefined
and the hook logs error("No session found") before returning.

That branch isn't an error: the provider's `location` redirect already
set on the response passes through the early return unchanged, so the
browser still returns to the app. Drop the misleading error log and
document why the branch is expected. No behavioral change.
@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

@mishafyi is attempting to deploy a commit to the Convex Team on Vercel.

A member of the Team first needs to authorize it.

@mishafyi

Copy link
Copy Markdown
Author

@erquhart — carrying your review over from #389 (that one is stuck closed after I botched a force-push; details there).

Verified against current main: you're right, the bare return keeps the redirect. The location header set upstream survives the early return, and the session branch reads that same header, so nothing gets swallowed. I couldn't reproduce the original dead-end because we've since dropped the crossDomain plugin from our own setup — so I'm deferring to your read rather than my stale 0.10.x observation.

Re-scoped to just what's actually wrong: the error("No session found") log on the no-newSession path (linkSocial et al.), which is an expected case, not an error. It's now a one-line removal plus a comment explaining the branch. src/plugins/cross-domain/index.test.ts still green (6/6).

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: get-convex/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 725a3d76-eb16-4f62-b669-83533d532244

📥 Commits

Reviewing files that changed from the base of the PR and between c628916 and 7fc4b84.

📒 Files selected for processing (1)
  • src/plugins/cross-domain/index.ts

📝 Walkthrough

Walkthrough

The cross-domain one-time-token callback now treats a missing newSession as an expected no-handoff scenario. It no longer emits a "No session found" error and continues by skipping one-time-token minting.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: suppressing a misleading error log on cross-domain callbacks without newSession.
Description check ✅ Passed The description directly explains the no-session callback path and the removal of the spurious log, matching the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

helixnebulatech added a commit to helixnebulatech/convex-better-auth that referenced this pull request Oct 8, 2026
Callbacks such as linkSocial and error redirects never create a session, so
there is no one-time token to mint. Skip silently instead of logging
"No session found" at error level.

Audit of upstream convex#417.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant