Ronin•2w ago

Race condition error upon await ctx.auth.getUserIdentity();[revised post]

Business Use Case: This condition was encountered by a new joiner trying to create an account on clarity and trying to join a space. Observed Behavior: The initial code for the getById function I wrote is as follows:

export const getById = query({
    args: {
        id: v.id("spaces")
    },
    handler: async(ctx, args) => {
        const identity = await ctx.auth.getUserIdentity();
        if(!identity){
            throw new Error("Not authenticated");
        }
        const user = await ctx.db
                .query("users")
                .withIndex("by_token", (q) => q.eq("tokenIdentifier", identity.tokenIdentifier))
                .unique();
        if(!user){
            throw new Error("User not found");
        }
        const member = await ctx.db
            .query("spaceMembers")
            .withIndex("by_space_id_user_id", (q) => 
                q.eq("spaceId", args.id).eq("userId", user._id)   
            )
            .unique();
        if(!member){
            return null;
        }
        return await ctx.db.get(args.id);
    }
});

export const getById = query({
    args: {
        id: v.id("spaces")
    },
    handler: async(ctx, args) => {
        const identity = await ctx.auth.getUserIdentity();
        if(!identity){
            throw new Error("Not authenticated");
        }
        const user = await ctx.db
                .query("users")
                .withIndex("by_token", (q) => q.eq("tokenIdentifier", identity.tokenIdentifier))
                .unique();
        if(!user){
            throw new Error("User not found");
        }
        const member = await ctx.db
            .query("spaceMembers")
            .withIndex("by_space_id_user_id", (q) => 
                q.eq("spaceId", args.id).eq("userId", user._id)   
            )
            .unique();
        if(!member){
            return null;
        }
        return await ctx.db.get(args.id);
    }
});

11 Replies

RoninOP•2w ago

To fix this I had to first check for the space instead of checking for authentication(we are using clerk for authentication vendor):

export const getById = query({
    args: {
        id: v.id("spaces")
    },
    handler: async(ctx, args) => {
        const space = await ctx.db.get(args.id);
        
        if (!space) {
            return null;
        }

        const identity = await ctx.auth.getUserIdentity();
        
        // If not authenticated, return null
        if (!identity) {
            return null;
        }

        const user = await ctx.db
            .query("users")
            .withIndex("by_token", (q) => q.eq("tokenIdentifier", identity.tokenIdentifier))
            .unique();

        if (!user) {
            return null;
        }

        const member = await ctx.db
            .query("spaceMembers")
            .withIndex("by_space_id_user_id", (q) => 
                q.eq("spaceId", args.id).eq("userId", user._id)   
            )
            .unique();

        if (!member) {
            return null;
        }

        return space;
    }
});

export const getById = query({
    args: {
        id: v.id("spaces")
    },
    handler: async(ctx, args) => {
        const space = await ctx.db.get(args.id);
        
        if (!space) {
            return null;
        }

        const identity = await ctx.auth.getUserIdentity();
        
        // If not authenticated, return null
        if (!identity) {
            return null;
        }

        const user = await ctx.db
            .query("users")
            .withIndex("by_token", (q) => q.eq("tokenIdentifier", identity.tokenIdentifier))
            .unique();

        if (!user) {
            return null;
        }

        const member = await ctx.db
            .query("spaceMembers")
            .withIndex("by_space_id_user_id", (q) => 
                q.eq("spaceId", args.id).eq("userId", user._id)   
            )
            .unique();

        if (!member) {
            return null;
        }

        return space;
    }
});

Expected Behavior : The function was expected to return space details without failing at the point

ctx.auth.getUserIdentity();
        if(!identity){
            throw new Error("Not authenticated");
        }

ctx.auth.getUserIdentity();
        if(!identity){
            throw new Error("Not authenticated");
        }

How this error can be reproduced : The error can be reproduced just by first trying to check for authentication before any other operation which triggers the race condition. The problem does not always occur frequently. We have worked with cases where User A would never see the issue and User B would always see the issue. The file in which this problem came is named spaces.ts. I have attached the file with the code

spaces.ts

ballingt•2w ago

@Ronin it sounds like sometimes the user is not authenticated, unrelated to any potential race condition. Your fix involves returning null if the user is not authenticated instead of throwing an error.

RoninOP•2w ago

I did that when I went with the approach to check for space first, are you suggesting going back to the way the function was written before and change the return null instead of throwing an error

ballingt•2w ago

I think there's no race condition here You can check for space first or second, doesn't matter, the important thing is deciding how to deal with the user not being authenticated. This code will not throw an error if the user is not authenticated, it will just return null

const identity = await ctx.auth.getUserIdentity();

// If not authenticated, return null
if (!identity) {
    return null;
}

const identity = await ctx.auth.getUserIdentity();

// If not authenticated, return null
if (!identity) {
    return null;
}

This code will throw an error if the user is not authenticated

const identity = await ctx.auth.getUserIdentity();
if(!identity){
    throw new Error("Not authenticated");
}

const identity = await ctx.auth.getUserIdentity();
if(!identity){
    throw new Error("Not authenticated");
}

RoninOP•2w ago

didn't adding a delay gave time to return authentication verification beacause before that even if someone is loggedin the auth was not working for them?

ballingt•2w ago

The delay doesnt matter in a query, queries and mutations run all at the same timestamp so can't have auth change in the middle of running them. But it's hard to tell from your code, because you're doing different things in these two examples the important difference is throwing an error vs returning null return null; vs throw new Error("Not authenticated");

RoninOP•2w ago

Okay, thanks Tom, let me take some time to find another example that more isolates our perceived race condition. I'll create a new support case if needed. But one general question: how should I handle the UserIdentity call? Should I throw an Error or return a null?

Matt Luo•2w ago

Hi Tom, to give some more context, this is an important question because we want to remove all other factors in this investigation and provide you an isolated example of our perceived race condition. As of now, we consistently create production issues upon a ctx.auth.getUserIdentity() call. We want to adhere to the best practice. And then find a pristine example for you. As of right now, we cannot invite users to the product because our getUserIdentity() isn't reliable

Matt Luo•2w ago

The correct way to call getUserIdentity()?

const identity = await ctx.auth.getUserIdentity();
if (identity === null) throw new Error("Unauthenticated");

const identity = await ctx.auth.getUserIdentity();
if (identity === null) throw new Error("Unauthenticated");

I see this code in https://docs.convex.dev/auth/functions-auth And in this Stack article, https://stack.convex.dev/testing-authenticated-functions-from-the-dashboard Ronin's workaround solution was to 1) remove throw new Error 2) add a query to spaces table before getUserIdentity(). We do not know why this workaround solution works.

ballingt•2w ago

@Ronin

But one general question: how should I handle the UserIdentity call? Should I throw an Error or return a null?

It depends on what you want, if you dont' think this query shuld ever fire unless the user is authenticated (so you're protecting the component that uses the useQuery() hook with a <Authenticated> React component importd from "convex/react"; like shown here https://docs.convex.dev/auth/clerk#logged-in-and-logged-out-views) then throwing an error seems good. But it sounds like this is happening to you sometimes, so if you need to run the query and don't want the error on the client side, then returning null works well.

Convex & Clerk | Convex Developer Hub

Clerk is an authentication platform providing login via

ballingt•2w ago

Sorry for the delay here. Re

As of now, we consistently create production issues upon a ctx.auth.getUserIdentity() call.

I think the issue to focus on is that sometimes this query is called before a user has authenticated. There are a few ways this can happen: if you're using React, the most common is that a useQuery() hook is called in a component that renders before the JWT from Clerk, Auth0, or Convex Auth, etc. has been sent to Convex. @Matt Luo @Ronin for either of you, let me know if there's a good time to hop on a call and look at this code together, or you can share access and I can write up some suggestions. re whether this is the correct way to call getUserIdentity(),

const identity = await ctx.auth.getUserIdentity();
if (identity === null) {
  throw new Error("Unauthenticated");
}

const identity = await ctx.auth.getUserIdentity();
if (identity === null) {
  throw new Error("Unauthenticated");
}

this is the correct way if you believe this situation could not possibly happen, and want to know about it as an error if it does. That's what's happening now. This only makes sense if you limit on the frontend when this Convex Query function is called to after the user has been authenticated.

Ronin's workaround solution was to 1) remove throw new Error 2) add a query to spaces table before getUserIdentity(). We do not know why this workaround solution works.

The relevant part here is 1), if you don't want this query to throw an error then remove throw new Error

Race condition error upon await ctx.auth.getUserIdentity();[revised post]

Did you find this page helpful?