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

Re-export ApiError from google-cloud/common to ease usage downstream #1268

Closed
MasterOdin opened this issue Sep 6, 2023 · 1 comment · Fixed by #1271
Closed

Re-export ApiError from google-cloud/common to ease usage downstream #1268

MasterOdin opened this issue Sep 6, 2023 · 1 comment · Fixed by #1271
Assignees
Labels
api: bigquery Issues related to the googleapis/nodejs-bigquery API. 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

@MasterOdin
Copy link

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

In our code, we want to check if an error that's returned by the BQ library is an ApiError, and if so, we want to re-use properties off it.

Within our code, we have something like this for getting a user's datasets:

import { BigQuery } from '@google-cloud/bigquery';
import { ApiError } from '@google-cloud/common';

client = new BigQuery({ /* options */ });

// ....

client
  .getDatasets()
  .then(([datasets]) => {
    // do something with the datasets
  })
  .catch((e) => {
    if (e instanceof ApiError) {
      if (e.code === 403 || e.code === 404) {
        // show user permission error
      }
    }
    // show user generic error message
  });

where to make the instanceof ApiError check work, we were adding @google-cloud/common to our dependencies and importing from there. We ended up with a bug where @google-cloud/bigquery got updated and so the version of @google-cloud/common in our dependencies no longer matched what @google-cloud/bigquery used, and the instanceof ApiError check now failed.

Describe the solution you'd like
A clear and concise description of what you want to happen.

It'd be nice if @google-cloud/bigquery re-exported ApiError so that the above code snippet could be rewritten as :

import { ApiError, BigQuery } from '@google-cloud/bigquery';

client = new BigQuery({ /* options */ });

// ....

client
  .getDatasets()
  .then(([datasets]) => {
    // do something with the datasets
  })
  .catch((e) => {
    if (e instanceof ApiError) {
      if (e.code === 403 || e.code === 404) {
        // show user permission error
      }
    }
    // show user generic error message
  });

and then the instanceof check will never fail.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

We could remove @google-cloud/common from our dependency list, and rely on it being available implicitly via how node_modules works, but that seems risky, and we could end up with being broken still depending on how yarn decides to hoist things. We've also looked to use duck typing, but that gives us less type strength.

Additional context
Add any other context or screenshots about the feature request here.

@MasterOdin MasterOdin 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 Sep 6, 2023
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/nodejs-bigquery API. label Sep 6, 2023
@loferris loferris self-assigned this Sep 11, 2023
@loferris
Copy link
Contributor

@MasterOdin I've re-exported the whole google-common package, which might make coding a bit more verbose, but will help us hopefully avoid having to do this piecemeal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/nodejs-bigquery API. 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

Successfully merging a pull request may close this issue.

2 participants