Skip to content

Conversation

@ljluestc
Copy link

Core idea: coalesce concurrent account creation for the same userId so that only one createAccount runs and all callers share the result.

  • In packages/api-core/src/util/buildContext.js:

    • Added a module‑level Map (accountCreationPromisesByUserId) keyed by userId to track in‑flight account creation promises.
    • Introduced ensureAccountForUser(context, userId):
      • First calls context.auth.accountByUserId(context, userId) and returns immediately if an account exists (fast path).
      • If no account exists:
        • If there is an in‑flight promise for this userId, it awaits and returns that same promise (no duplicate createAccount).
        • Otherwise it:
          • Starts a new context.mutations.createAccount(context.getInternalContext(), { emails, name, profile, userId }) call.
          • On error (e.g., unique index), re‑checks accountByUserId and only logs if the account is still missing.
          • Stores the promise in the map while in flight and deletes it in a finally block.
          • Returns the created or fetched account (or null).
    • Replaced the previous inline “lookup then maybe create” logic with:
      • account = await ensureAccountForUser(context, userId);
  • In packages/api-core/src/util/buildContext.test.js:

    • Kept existing tests for:
      • “properly mutates the context object without user”
      • “properly mutates the context object with user”
    • Added a new concurrency test:
      • coalesces concurrent account creation for the same user into a single createAccount call
      • Simulates 5 concurrent buildContext calls for the same new userId:
        • Uses a shared createdAccount variable.
        • Mocks accountByUserId to return createdAccount.
        • Mocks createAccount to:
          • Wait briefly (async delay),
          • Set createdAccount = { _id: "CONCURRENT_ACCOUNT_ID", userId },
          • Return it.
        • Provides a minimal getInternalContext stub on the test context:
          • getInternalContext() { return { ...this, isInternalCall: true }; }
        • Asserts that:
          • createAccount is called exactly once.
          • All 5 contexts receive the same account, accountId, and userId.

Other potential solutions considered:

  • Distributed lock (Redis / Mongo lock collection / Redlock):
    • Would give cross‑process guarantees, but adds infra complexity and configuration.
    • Rejected for this PR in favor of a simpler, in‑process fix that is compatible with the existing plugin model and can be extended later if needed.
  • Changing the behavior to surface an error to clients instead of swallowing:
    • Would alter established semantics and could be breaking for existing clients that rely on current behavior.
    • Out of scope for this bugfix; we preserved the existing error‑swallowing, focusing strictly on removing the race.

@changeset-bot
Copy link

changeset-bot bot commented Nov 22, 2025

⚠️ No Changeset found

Latest commit: 8b19687

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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