Bug in createSessionStorage: expires not passed to updateData and createData

[naveed-fida]

What version of Remix are you using?

1.16.1

Are all your remix dependencies & dev-dependencies using the same version?

• Yes

Steps to Reproduce

I've been working on a remix app and one of the customizations I did was to change the session workflow in Blues stack to using my app's Postgres database to store sessions. That means I had to use the createSessionStorage function to create my own CRUD for the session store. It all went fine, but I noticed that my expiresAt field in the session model wasn't getting populated, even though I've been using the expires argument to populate it. Here's how I'm implementing my own session store:

import { createSessionStorage } from "@remix-run/node";
import type { Cookie, CookieOptions } from "@remix-run/node";
import { prisma } from "~/db.server";
import type { Session } from "@prisma/client";
import type { SessionData } from "@remix-run/node";
import invariant from "tiny-invariant";

interface StoreGeneratorArg {
  cookie: Cookie | CookieOptions;
}

function createDBSessionStorage({ cookie }: StoreGeneratorArg) {
  return createSessionStorage<SessionData & Pick<Session, "userId">>({
    cookie,
    createData: async (data, expires) => {
      const { userId } = data;
      invariant(userId, "userId must be set");
      invariant(expires, "expires must be set");

      const session = await prisma.session.create({
        data: {
          userId,
          expiresAt: expires,
        },
      });

      return session.id;
    },
    readData: async (id) => {
      return await prisma.session.findUnique({ where: { id } });
    },
    updateData: async (id, data, expires) => {
      const userId = data.userId;
      invariant(userId, "userId must be set");
      invariant(expires, "expires must be set");

      await prisma.session.update({
        where: { id },
        data: {
          userId,
          expiresAt: expires,
        },
      });
    },
    deleteData: async (id) => {
      await prisma.session.delete({ where: { id } });
    },
  });
}

export { createDBSessionStorage };

I added the invariant for expires just to see whether it's being populated or not, and sure enough, it raises the error.

Here's how I'm using the commitSession function:

export async function createUserSession({
  request,
  userId,
  remember,
  redirectTo,
}: {
  request: Request;
  userId: string;
  remember: boolean;
  redirectTo: string;
}) {
  const session = await getSession(request);
  session.set(USER_SESSION_KEY, userId);

  return redirect(redirectTo, {
    headers: {
      "Set-Cookie": await sessionStorage.commitSession(session, {
        expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 7), // 7 days
      }),
    },
  });
}

Expected Behavior

Given the code above, I was expecting the expires argument to the updateData function to be populated since I'm providing an expires option to sessionStorage.commitSession. I'm basing this assumption on the following quote from this doc page:

The expires argument to createData and updateData is the same Date at which the cookie itself expires and is no longer valid. You can use this information to automatically purge the session record from your database to save on space, or to ensure that you do not otherwise return any data for old, expired cookies.

Actual Behavior

However, as I mentioned above, the expires argument is undefined. This led me to take a look at the relevant code in the remix codebase and came across this function in this file

export const createSessionStorageFactory =
  (createCookie: CreateCookieFunction): CreateSessionStorageFunction =>
  ({ cookie: cookieArg, createData, readData, updateData, deleteData }) => {
    let cookie = isCookie(cookieArg)
      ? cookieArg
      : createCookie(cookieArg?.name || "__session", cookieArg);

    warnOnceAboutSigningSessionCookie(cookie);

    return {
      async getSession(cookieHeader, options) {
        let id = cookieHeader && (await cookie.parse(cookieHeader, options));
        let data = id && (await readData(id));
        return createSession(data || {}, id || "");
      },
      async commitSession(session, options) {
        let { id, data } = session;

        if (id) {
          await updateData(id, data, cookie.expires);
        } else {
          id = await createData(data, cookie.expires);
        }

        return cookie.serialize(id, options);
      },
      async destroySession(session, options) {
        await deleteData(session.id);
        return cookie.serialize("", {
          ...options,
          expires: new Date(0),
        });
      },
    };
  };

And it all makes sense. Instead of providing cookie.expires to updateData, the commitSession function should be providing the expiry it receives in the options object. I could initialize my cookie or cookie data to have an expires field, but that would be a static date that gets created once. So that doesn't seem like a valid option.

[brophdawg11]

Fixed by #6598 and will be released in Remix v2

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-f77588e-20230826 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 2.0.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!