Add proper error handling and wrapping

[dankochetov]

No description provided.

[kibertoad]

What is missing?

[probablykasper]

Is this for returning errors, so we can prevent bugs from forgetting to handle exceptions and from incorrectly guessing the error type?

[fermentfan]

This would be super helpful for our code quality. I noticed this when Drizzle threw me an error, when I tested our API with a wrong resource ID to test if the 404 works. Postgres was already mad, because the wrong ID wasn't a valid UUIDv4. Catching this now and throwing an appropriate 404 is kinda smelly.

Error codes would be nice to have!

[swelham]

Another example of how error handling could improve code quality is with errors raised from constraints.

For example, when see a unique containst error, we get a fairly raw looking error object in our try..catch(e).

{
  code: '23505'
  column: undefined
  constraint: 'users_email_index'
  dataType: undefined
  detail: 'Key (email)=([email protected]) already exists.'
  file: 'nbtinsert.c'
  hint: undefined
  internalPosition: undefined
  internalQuery: undefined
  length: 231
  line: '664'
  name: 'error'
  position: undefined
  routine: '_bt_check_unique'
  schema: 'public'
  severity: 'ERROR'
  table: 'users'
  where: undefined
  message: 'duplicate key value violates unique constraint "users_email_index"'
  stack: <stacktrace_goes_here>
}

We then need to check that certain properties exist with specific values to determine what type of error it was.

A few options are:

// Check the constraint name or message
e.constraint === 'users_email_index';
//or
e.message === 'duplicate key value violates unique constraint "users_email_index"';

// Checking the detail field with a regex
const rx = new RegExp(/Key \(email\).*already exists/);
rx.test(e.detail);

// Checking more internal specifics
e.routine === '_bt_check_unique';

It would be really nice to have some higher level type that hides the internals and allows for simpler use within a catch statement.

[algora-pbc]

💎 $50 bounty created by @john-griffin
👉 To claim this bounty, submit your pull request on Algora
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to drizzle-team/drizzle-orm!

[rishi-raj-jain]

What's the exactly scope of this? @john-griffin

[kesh-007]

Is the issue still open? I am interested in working on this.

[swelham]

Hey. Yes this is still open.

From this issue we (@john-griffin and myself) are hoping to achieve a higher level error to work with that has better support for typescript. For example, looking at my comment above, all of the examples rely on checking strings for certain values in a way that doesn't allow us to capture typos at compile time.

I don't have a firm solution and maybe the drizzle team can make some recommendations here, but if we could achieve a type safe way to check for expected database errors, that would achieve the scope of the bounty.

Here are some rough ideas that we have discussed, but are by no means prescriptive.

// Generic error class
class QueryError extends Error {
  type: "NonNull" | "UniqueConstraint" | "CheckConstraint" | "ForeignKey" // etc...
  table: string; // Can this be typed to all known tables
  field: string; // Can this by typed to all known fields
  message: string;
}

// or

// Error type specific error classes
class UniqueConstraintError extends Error {
  table: string; // Can this be typed to all known tables
  field: string; // Can this by typed to all known fields
  message: string;
}
class NonNullError extends Error {
  table: string; // Can this be typed to all known tables
  field: string; // Can this by typed to all known fields
  message: string;
}

Something like this would allow checking for errors without dealing directly with the error class from the pg module and could potentially allow better typing.

catch (e) {
  if (e instanceof QueryError) {
    if (e.type === 'unique' and e.field === 'email') { ... }
    if (e.type === 'unknown_type' and e.field === 'email') { ... } // < type error
  }
}

// or

catch (e) {
  if (e instanceof UniqueConstraintError) {
    if (e.field === 'email') { ... }
    if (e.field === 'unknown_field') { ... } // < type error
  }  
}

Again, happy to defer to the drizzle team for API specific guidance.

[Angelelz]

Those working on this one should do the slash command for algora.

[Neeraj319]

add error classes fast bro

[afogel]

any updates on where this is prioritized in roadmap? Seems like someone might be open to working on it, as long they get guidance from drizzle team on what would work best for drizzle's API? @Angelelz

[dinogomez]

Bumping @afogel 's question, good error handling would be great for displaying the errors on the forms.

[StepanMynarik]

Yet another unique constraint user here. Very much +1

[bytekai]

+1

[nantokaworks]

+1

[ThomasAunvik]

+1 It would really help alot with debugging and error reporting if you had a usable stacktrace. Had a time figuring out why that it was my /[anything]/layout.tsx was causing the issue, and not the /layout.tsx or the /(main)/layout.tsx.

 ⨯ PostgresError: invalid input syntax for type uuid: "some-404-page"
    at Socket.emit (node:events:519:28)
digest: "2286078463"
 GET /some-404-page 500 in 97ms

[afogel]

@Angelelz just a gentle bump reminder here -- any additional guidance the team can give here?

[creadevv]

For the ones having issues with invalid uuids, I have kind of a work around.
I am using uuid_v1(), and those strings are always 32 letters/numbers and 4 hyphens, so 36 characters.

I am using Sveltekit and put this in my +page.server.ts:

 if (params.uuid.length !== 36) {
	throw error(404, 'Not found here');
}

const [event] = await db.select().from(eventTable).where(eq(eventTable.uuid, params.uuid));

if (!event) {
	throw error(404, 'Not found here');
}

You can even add some regex to check if there is a combination of 4 hyphens and 32 letters/numbers.

The best solution would still be a way to handle those errors.

[cellulosa]

I'm working with superforms and trpc.
For the moment I'm handling errors like so:

// +page.server.ts

export const actions: Actions = {
	default: async ({ request, fetch }) => {
		const form = await superValidate(request, zod(schema));

		if (!form.valid) {
			return fail(400, { form });
		}

		try {
			const trpc = trpcOnServer(fetch);
			const [newClient] = await trpc.clients.save.mutate({ name: form.data.clientName });
			setMessage(form, `The project was created correctly.`);

		} catch (error) {
			if (error instanceof TRPCError || error instanceof TRPCClientError) {
				if (error.message == 'duplicate key value violates unique constraint "client_name_unique"')
					setError(form, 'clientName', 'Client name already exists.');
			} else {
				setMessage(form, 'Could not create the project.', {
					status: 400
				});
			}
		}

		return { form };
	}
};

but would be good to be able to avoid hardcoding it

[jonnicholson94]

+1 on this - would be great!

[RobertMercieca]

+1 would indeed be nice to have

[cybercoder-naj]

+1 love to see good error handling from drizzle

[thebergamo]

It would be really good to have at least some simple examples on error handling in the docs.

[probablykasper]

I use this helper function to help alleviate the pain:

type QueryError = Error & { code?: unknown }

export async function query<D, P extends QueryPromise<D>>(promise: P) {
	try {
		const result = await promise
		return { data: result, error: null }
	} catch (e) {
		if (e instanceof Error) {
			return { data: null, error: e as QueryError }
		} else {
			throw e
		}
	}
}

Example usage:

const { data, error } = await query(
	db.insert(users).values({
		email,
	}),
)
if (error?.code === '23505' && error.message === 'duplicate key value violates unique constraint "users_email_unique"') {
	return fail(400, 'Email already in use')
} else if (!data) {
	throw error
}
console.log(data)

[ayoubkhial]

We can also rely on the error code property:

Unique constraint:

try {
  return await this.db.insert(schema.users).values(user).returning();
} catch (error) {
  const code: string = error?.code;
  // Unique constraint:
  if (code === '23505') {
    const detail: string = error?.detail;
    const matches = detail.match(/\((.*?)\)/g);
    if (matches) {
      const [key, value] = matches.map((match) => match.replace(/\(|\)/g, ''));
      throw new BadRequestException(`An entry with the value '${value}' already exists for the field '${key}'.`);
    }
  } 
  // Not-Null constraint
  else if (code === '23502') {
    const table: string = error?.table;
    const column: string = error?.column;
    throw new BadRequestException(`The column '${column}' in table '${table}' cannot be null.`);
  }
}

But for other validation constraints, like VARCHAR(50), the error details are a bit limited:

{
  message: value too long for type character varying(50)
  length: 98,
  severity: 'ERROR',
  code: '22001',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'varchar.c',
  line: '638',
  routine: 'varchar'
}

[kedom1337]

+1 much needed feature

[hi-reeve]

+1 for this

[waynehamadi]

I ended creating my own middleware for drizzle, and it gives me a lot of control, including the ability to finally see errors in my application code without having to put try catch on all my queries:
https://gist.github.com/waynehamadi/571b1e9a6d00adcae33fd25eccf3833a

the code is very opinionated for a remix/express app, so really just to give inspirations more than a clear guide

[felixgeissler]

+1 here as well.

[elieven]

bump

[wmtrinu]

+1, I need to do proper error handling,thanks!

[murolem]

bump

[Naveenravi07]

+1

[aparx]

+1

[omar-tabbush]

+1

[mkpanq]

Just started to create logic for handle errors from drizzle in my app, so...

+1

[thanhlq]

+1

[Ericnr]

+1

[stefanprobst]

please stop the "+1" comments and use the thumbs-up reactions button. thanks.

[RickVM]

For those using postgres,we use the following workaround that might be of some inspiration for implementing your own (workaround)

Here's how I built something to identify uniqueConstraintErrors.

import pg from 'postgres';

const isUniqueConstraintError = (error: unknown): boolean => {
  /** https://github.com/porsager/postgres/pull/901 */
  // eslint-disable-next-line import/no-named-as-default-member
  return error instanceof pg.PostgresError && error.code === '23505';
};

export default isUniqueConstraintError;

import isUniqueConstraintError from '@/lib/database/errors/isUniqueConstraintError';

// Insert item into db
try {
  // Insert item into the database with the given data and max position
  await db.insert(cars).values({ ...data });
} catch (error) {
  // Check if the error is a unique constraint error
  if (isUniqueConstraintError(error)) {
    // handle however you want....
  } else {
    // Handle other errors
    ....
  }
}

Ofcourse, you can use the PostgresError to get more details.

[L-Mario564]

I'm beginning work on this issue. This is by no means the finalized API, but here's what I got at the moment:

import { is } from 'drizzle-orm';
import { PgQueryError, ERROR } from 'drizzle-orm/pg-core';

try {
  await db.insert(users).values({ email: '[email protected]' })
} catch (err) {
  if (is(err, PgQueryError)) {
    // This is a PG error. This class applies regardless of driver being used

   if (err.code === ERROR.INTEGRITY_CONSTRAINT_VIOLATION.UNIQUE_VIOLATION) {
     // Unique constraint violation error
    error(401, 'a user with that email address already exists'); 
   }
  } else {
    // Some other kind of error
  }
}

The PgQueryError class would contain all the details present in the native error thrown by the DB, alongside some additional utils like getting the column names of the relevant columns (for integrity constraint violations) and the query build by Drizzle with its params.

Some possible additions:

• Query execution time (could go with [FEATURE]: i want SQL execution duration at logger. #2605).
• A global onError hook upon initializing the driver:

const db = drizzle(postgres, {
  onError: (err) => { /* Do stuff */ }
})

Any feedback is welcome.

[probablykasper]

With that implementation, I'll write something like this when I wanna return the value and error:

import { is } from 'drizzle-orm';
import { PgQueryError } from 'drizzle-orm/pg-core';

// Use .catch() instead of try...catch so we keep the type of the returned data
const result = await db.select().from(users).catch((error) => {
  // Since there's no type safety here, you have to guess the error type. If you
  // forget to do the check correctly, or if you miss that a future change, your
  // users will hopefully let you know
  if (is(error, PgQueryError)) {
    // Since we're in a closure, we need to return it here so we can return it again later
    return { error };
  } else {
    // IMPORTANT, DO NOT FORGET: `error` can be anything, so make sure you don't
    // silently ignore it
    console.error('Unexpected error', error);
    throw error;
  }
})
if ('error' in result) {
  return { error: result.error };
}
return result;

I would much prefer error-by-value:

// result type: { data: T, error: null } | { data: null, error: { ... } }
const result = await db.select().from(users);
if (result.error) {
	return { error: result.error};
}
return result.data;

[L-Mario564]

With that implementation, I'll write something like this when I wanna return the value and error:

That would be a big breaking change for Drizzle's API, not to mention that error can't be fully type safe, as you may encounter Drizzle errors, driver errors, vendor-specific DB errors, standard DB errors, Node/Bun/Deno/Hermes errrors, etc., many of which aren't documented to begin with.

Maybe we could implement an API like that via an abstraction provided by Drizzle, but that'd be a separate thing and won't consider this within the scope of this issue, since you can make your own abstractions anyways.

[probablykasper]

Since the issue is about proper error handling, I was hoping that meant moving away from error-guessing. In my opinion it's important for writing reliable software, especially when dealing with databases. Should I open a new issue about it?

not to mention that error can't be fully type safe

I'd say that error-by-value is always type-safe (*by TypeScript standards). Unexpected errors should still be exceptions, like panics in other languages. If a dependency throws an exception that Drizzle can't reliably know the type of, it can have a type like for example { code: 'pg_core', details: unknown }

[darrenkeen]

For those using postgres,we use the following workaround that might be of some inspiration for implementing your own (workaround)

Here's how I built something to identify uniqueConstraintErrors.

import pg from 'postgres';

const isUniqueConstraintError = (error: unknown): boolean => {
/** porsager/postgres#901 */
// eslint-disable-next-line import/no-named-as-default-member
return error instanceof pg.PostgresError && error.code === '23505';
};

export default isUniqueConstraintError;

import isUniqueConstraintError from '@/lib/database/errors/isUniqueConstraintError';

// Insert item into db
try {
// Insert item into the database with the given data and max position
await db.insert(cars).values({ ...data });
} catch (error) {
// Check if the error is a unique constraint error
if (isUniqueConstraintError(error)) {
// handle however you want....
} else {
// Handle other errors
....
}
}

Ofcourse, you can use the PostgresError to get more details.

Weirdly, error instanceof pg.PostgresError returns false for me. Can't see severity_local property in my error 🤔 Supabase not returning what a normal postgres DB would perhaps?