gaxios icon indicating copy to clipboard operation
gaxios copied to clipboard

Feature request: optional error throwing

Open TeemuKoivisto opened this issue 2 years ago • 1 comments

Hi, I recently came accustomed to using Gaxios through googleapis (specifically drive_v3). Seems all Promises are wrapped as GaxiosPromises which is okay but I have a minor nitpick to make.

While it's the norm to throw errors in JS I think it's mostly bad practise as this will often just crash the app and you can't type the errors as you could with normal returns.

I myself have been using this utility type:

export type Ok<T> = {
  data: T
}
export type Error = {
  err: string
  code: number
}
export type Maybe<T> = Ok<T> | Error

to wrap all my error-producing code (mostly asynchronous) to gain full type-safety and proper error messages - akin to Go. I can then use them as such:

  const resp = await driveService.listDrives(client);
  if ("err" in resp) {
    return next(new CustomError(resp.err, resp.code));
  }
  res.json(resp.data);

Now, I'm not suggesting you have to follow this schema (I used to use ok: boolean to check for errors but scrapped it once I saw 'data' in resp works as well). But since you are wrapping Promises with your own type which includes both, success and error types, I'd love to be able to use those directly instead of catching the error, typing it manually and then doing my own thing on top of it causing just a lot boilerplate.

Ideally, there was a parameter / wrapper / utility method to allow returning an optional type which is either GaxiosResponse<T> | GaxiosError. Then I could myself check that for errors and not worry about typings and such.

What urged me to write this in the first place was noticing passing expired credentials caused Gaxios to throw an error while an invalid request would instead return normally (although with non 200 code). Which is just inconvenient as hell as I have to do both, wrap the request with try-catch and check the response for non 200 status.

EDIT: Also I must say GaxiosError is pretty awkward to use. Why the stack is not serialized into a regular JSON object? I'm now manually slicing the error message from it.

TeemuKoivisto avatar Feb 06 '23 13:02 TeemuKoivisto

Huh okay, it wasn't too difficult to come up with this which seems to work:

async function wrapGaxios<T>(promise: GaxiosPromise<T>): Promise<Maybe<T>> {
  try {
    const res = await promise
    if (res.status !== 200) {
      return { err: res.data as any, code: res.status || 500 }
    }
    return { data: res.data }
  } catch (err: any) {
    if (err instanceof GaxiosError) {
      const code = parseInt(err.code || '500')
      return { err: err.message, code }
    }
    return { err: err, code: err?.status || 500 }
  }
}

And then using it eg

const resp = wrapGaxios(drive.files.get({
  fileId: 'root',
  fields: 'id, name'
}))

TeemuKoivisto avatar Feb 06 '23 13:02 TeemuKoivisto

Hey @TeemuKoivisto, thanks for your feature request. Your wrapGaxiosError function should work for your needs. Today, it isn't a common convention to return errors without throwing (except for callbacks).

danielbankhead avatar May 15 '24 19:05 danielbankhead