Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: optional error throwing #523

Open
TeemuKoivisto opened this issue Feb 6, 2023 · 1 comment
Open

Feature request: optional error throwing #523

TeemuKoivisto opened this issue Feb 6, 2023 · 1 comment
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@TeemuKoivisto
Copy link

TeemuKoivisto commented Feb 6, 2023

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 TeemuKoivisto added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 6, 2023
@TeemuKoivisto
Copy link
Author

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'
}))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants