find vs findOrThrow?

[romeerez]

Currently, methods like find, take, findBy, get are throwing when no records found.

And this enables more concise code:

const one = await db.table.find(123)

const two = await db.table.findBy({ key: 'value' })

Also, it works in a same way when it's used inside select. The following code will throw if there is no related record:

const result = await db.table.find(123).select('id', (q) =>
  q.related.select('title').take()
)

Does this feel unexpected? Would you prefer more explicit OrThrow :

const one = await db.table.findOrThrow(123)

const two = await db.table.findByOrThrow({ key: 'value' })

const result = await db.table.findOrThrow(123).select('id', (q) =>
  q.related.select('title').takeOrThrow()
)

So on one hand, find is concise and looks neat, findOrThrow creates visual noise. And, in my opinion, throwing by default is preferred as you don't have to think about the not found case. On other hand, findOrThrow is more obvious and forces user to think if they really want a throw here or if they should findOptional.

Perhaps, the best way here is to allow customizing this.

[quinnvaughn]

I don't like the default throwing. I use orchid in a graphql server and I return my errors as objects, so I don't want to throw a random error. I always end up having to reach for "optional" so I can then manually return a specific object type.

[bingtsingw]

I found many ORMs provide two methods, find to return Object | null, findOrThrow to return Object, it seems to like a de-facto in my experience.
prisma adapt to this design in version 4

In my developing experience, I found there are some cases I do not want a default throw,

const user = db.user.find(1)

if (user) {
// some logic ...
} else {
// some logic ....
}

[romeerez]

I can't change find to return Object | null because it may lead to problems, for instance:

app.get('/thing/:id', async (req, res) => {
  const thing = await db.thing.find(req.params.id)
  res.send(thing)
})

The client (frontend) may easily assume that this endpoint always returns Object, and in development mode, we can easily miss the problem, but later in production, it may cause a bug. In addition, handling null case on frontend would require more code, while throwing error can be handled in one centralized place, in axios interceptor or something. If any request returns NotFound error then display "Not found" page, or redirect to somewhere, or display error toast.

What I'm thinking about is if it is worth renaming find to findOrThrow for clarity, so there won't be find at all, and you'll have to choose between findOrThrow or findOptional - maximum clarity, no confusion. A bit more letters to type.

In my developing experience, I found there are some cases I do not want a default throw,

IMO in 99% of cases, we just throw a "not found" error when not found, so it's done automatically.

Could you give an example of what else can be done in the not found case?

const user = db.user.find(1)

if (!user) {
  // ...what can be done in such case?
}

If we want to create a new user if it is not found, there is a orCreate method for this case.

[bingtsingw]

I totally agree that in 99% of cases, I use the findOrthrow method, and orCreate can handle most of the rest, but in rare circumstances, I may need extra initialization steps for creation, so I will choose to find first and then use a condition to decide what to do.

I think your concern "find() return null may lead to unexpected production bug" sounds reasonable, so I'm voting throw by default now(by the way, how to change the vote choice)

A find and findOptional combination looks good to me, use find() for 99% of cases with easy typing, use findOptional for rare cases where you really want to handle null by hand

[romeerez]

Looks like there is no way to change the vote. But you may bring more people into Orchid so they can vote :)

Nice point about initialization code before creating, I should add support for a callback to this method for this.

[IlyaSemenov]

While I prefer the current API where find throws, I actually have lots of repetitive code going like:

const game = await db.game.select(...).findOptional(gameId)
if (!game) {
  throw createError({ statusCode: 404, message: "Game not found." })
}

Ideally, I'd like this be somehow streamlined. I'm not convinced it should be done on the ORM side of things, but if it will, I suppose it could be like:

const http404 = () => createError({ statusCode: 404 })

...

const game = await db.game.select(...).find(gameId).orThrow(http404)

The callback could be passed some kind of context which it could use to actually build individualized messages, if it wants to:

const http404 = ({ query, error }) => createError({
  statusCode: 404,
  message: `${query.table} not found: ${error}` // just making it up
})

Again, I'm not saying ORM should do this. After all, I suppose I could simply go with:

const game = await db.game.select(...).findOptional(gameId)
assert404(game, `Game ${gameId} not found.`) // Not much worse and perhaps more flexible.

But then it goes back to using findOptional most of the time.

[romeerez]

In one or another way, centralized error handling can be added to any project, is available in every framework, be it graphql or rpc.

You could catch errors, differentiate them using instanceof, ORM's error objects have query object on them that has table.

query.table has string | undefined type, but it's undefined only in such a rare cases that are hard to imagine, you could use it for error message.

Game ${gameId} not found.

With centralized err handling, it will be hard or unachievable to embed the id, but is it really needed? If the id is meant for developers, they can easily figure out the id in other ways. It can't be meant for users, isn't it, users shouldn't be bothered with ids of their entities. In some cases, they might want to see product SKU or some kind of a tracking number of their parcel, but not the id.

Just checked on amazon marketplace, if you change product id in url it simply says "Page is not found", it doesn't even say "Product is not found", and it's alright.

Thinking of Product SKU and special parcel ids, what if you could add a not found error constructor to a specific table class? Then you could customize error message for a specific table, and it could include SKU or other useful info.

---

I prefer to handle errors in one place, reducing error handling in the app code, and not caring much of error messages, but of course this strategy is not perfect for all kinds of projects.

As long as business isn't concerned much of error messages, and even billion-worth platforms can feed you with "something went wrong", this strategy works well and is good enough.

But it's not enough in some cases, and it's not about error messages, but more about expected/unexpected errors.
If id is coming from a client, it's expected that sometimes it throws not found.
But in your own logic flows when id is coming from some other part of the app, it may be a bug that it is not found.

So one is expected, other is not expected, it's important to report all unexpected errors to your error monitoring platform, and to not flood it with expected errors that aren't a bug.

I'm not sure if orThrow(customError) is a good way for differentiating errors, because when you have to write like 100 of them, I'd definitely forget to add it in a few placed, and would pick a wrong custom error in other few places.

What is the best way? Not "depends", there must be an answer. Best way is when you can't forget to add something or do it slightly different by mistake, without noticing.

So I'd go with centralized handling anyway:

• around public-facing controllers, ORM's NotFoundError would go directly to clients.
• around jobs, some internal logical parts, microservices-facing controllers, not finding a record by id is a hint of something being wrong, and it's reported.
• findOptional is still here for handling not found in a specific way.