prisma: findFirst with `undefined` value shouldn't return data

This behaves as intended, you can read more here: https://www.prisma.io/docs/concepts/components/prisma-client/null-and-undefined#use-case-null-and-undefined-in-a-graphql-resolver. However, we can update our JSDoc for all queries to explain the difference and point to the docs.

This would be good to include in big red text in Sequelize migration tips&tricks. We have just released Prisma to production, where we replaced thousands of lines of code from Sequelize. All in all Prisma is awesome, but this one thing introduced nasty bug. Of course due to our fault, but as the issue mentioned, this is not intuitive.

Can there be a flag where you can control the behaviour? The case is that it happens a lot in auth code etc where you do

   const device = await prisma.deviceModel.findFirst({ where: { deviceId } });

If by accident you pass undefined here, the random model is returned which is really dangerous in some cases.

It’s a nightmare. You need to add a huge red banner with an explanation to the documentation! I can only imagine how many bugs appear because of this “smart” design

This has the be one of the worse design decisions. We also just found a bug where findFirst returns another user’s record. The problem is it does this silently.

For anyone who is currently struggling with this issue. The following workaround could use prisma middleware to prevent any undefined appears in where. For me, this is handy that I could prevent to make mistakes which will let the undefined to sneaked in.

Write a custom prismaClient instance which include the middleware. The middleware will check the params.args.where and throw an Error if any undefined has been included.

import { PrismaClient } from '.prisma/client';

const prisma = new PrismaClient();
prisma.$use(async (params, next) => {
  if (hasUndefinedValue(params.args?.where))
    throw new Error(
      `Invalid where: ${JSON.stringify(params.args.where)}`
    );
  return await next(params);
});

export default prisma;

function hasUndefinedValue<T>(obj: T): boolean {
  if (typeof obj !== 'object' || obj === null) return false;
  for (const key in obj) {
    if (!Object.prototype.hasOwnProperty.call(obj, key)) continue;
    const value = obj[key];
    if (value === undefined) return true;
    if (typeof value === 'object' && !Array.isArray(value))
      if (hasUndefinedValue(value)) return true;
  }
  return false;
}

Then you could export and use the prismaClient instance as usual. It will also be better to have the test case to verify the middleware is working fine:

describe('with undefined', () => {
  it('should throw Error', async () => {
    const error = new Error('Invalid where: {}');
    await expect(
      prisma.user.findUnique({ where: { email: undefined } })
    ).rejects.toThrow(error);
  });
});

Hope this help somebody!

This has the be one of the worse design decisions. We also just found a bug where findFirst returns another user’s record. The problem is it does this silently.

And not just query, but it could happen in update/delete as well. Really dangerous.

Appreciate the response. I figured that was the case and while not the ideal people in this thread want, I understand, because yeah… this would be a massive breaking change. Appreciate ya chiming in here so that way in the future people at least know the current state on this from Prisma’s perspective!

Totally agree with this is a bad and dangerous design.

I still believe this is such a dangerous design: once again just imagine writing auth function that looks up user by email and hash and by accident it comes up empty. The first user is usually the admin…

It would be worth it to change it with breaking change and add configuration option to opt in to the old behavior.

It’s just dangerous and not only bad design. For now the big red alert anywhere near filters documentation should be posted.

No. Someone made this design decision a long time ago, we can only change it with a breaking change, and we are not eager to do that right now. We are aware it is a problem and are taking comments like these into account. But I am also aware that engaging here will not really make any of you happy, as you want to get the behavior reverted or a big red banner at the top of our docs - both which will not happen.

There also a lot of other issues about this, for example https://github.com/prisma/prisma/issues/11021 or https://github.com/prisma/prisma/issues/20169. You can leave you upvotes on these.

Totally dangerous. They should listen to users’ feedback and change the design.

Respectfully this is a ticking time bomb. It’s a nasty post-mortem waiting to go viral on HN. We’ve been using Prisma for over a year at this point and have read a good chunk of this documentation. Somehow we’re only learning about this behavior now. I’m guessing the same is true for a lot of other users.

You can say it’s intended, but the design is really BAD, IMO.

Now, I have to check every single parameter passed to findFirst. This becomes ridiculous. And ppl can still easily make mistakes. It is especially dangerous when matching multiple fields when one undefined sneaked in.

Have just come across this and I was pretty surprised by it.

I would also describe it as dangerous, mostly because I’d guess that most people would not assume this behaviour, and will instead rely on error handling code after the query to check if nothing was found when an undefined id is passed, for example.

I think it would be great to introduce as a feature the ability to configure this behaviour off globally.

I can see an argument that typescript can eliminate this bug too. The reason I came across it was admittedly due to a lazy workaround on my part – I was asserting a query param was present in TS with the !, then the query param name changed, resulting in it being undefined. Really I should have used a zod-like runtime type guard. But, even in the case of using TS, examples like this of things not being quite perfect and undefineds sneaking in at runtime abound. I really think it’s better to have the additional failsafe here.

This is the solution: It’ll be available in a bit.