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

util.decorateRequest should confirm error is a missing project ID error #2069

Closed
gajus opened this issue Mar 9, 2017 · 3 comments
Closed
Assignees
Labels
core priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@gajus
Copy link

gajus commented Mar 9, 2017

A bug in the google-cloud package resulted in the following error:

Error: Sorry, we cannot connect to Cloud Services without a project ID. You may specify one with an environment variable named "GCLOUD_PROJECT". See https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/authentication for a detailed guide on creating an authenticated connection.
    at Object.<anonymous> (/Users/gajus/Documents/dev/applaudience/movie-editor/node_modules/google-cloud/node_modules/@google-cloud/common/src/util.js:55:29)
    at Module._compile (module.js:571:32)
    at Module._extensions..js (module.js:580:10)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/gajus/Documents/dev/applaudience/movie-editor/node_modules/babel-register/lib/node.js:152:7)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/gajus/Documents/dev/applaudience/movie-editor/node_modules/google-cloud/node_modules/@google-cloud/common/src/service-object.js:32:12)

The reason I am getting this error is this code block:

        try {
          authenticatedReqOpts = util.decorateRequest(
            authenticatedReqOpts,
            extend({ projectId: authClient.projectId }, config)
          );
        } catch(e) {
          err = util.missingProjectIdError;
        }
      }

Here catch assumes that any error that occurred in try is equivalent to missingProjectIdError.

Instead of blindly assuming what is the error, use instanceof check and a custom error constructor, e.g. https://github.com/gajus/xfetch/blob/0d8cd012354b0bb8b67162f74b2a9e126d3bdabb/src/attemptRequest.js#L52-L67.

Furthermore, what made it hard to debug is the lack stack trace (raised a separate issue #2068).

@stephenplusplus
Copy link
Contributor

What was the error that it mistook for a missing project ID error?

@gajus
Copy link
Author

gajus commented Mar 9, 2017

Raising another issue for that.

@gajus
Copy link
Author

gajus commented Mar 9, 2017

@stephenplusplus #2070

@stephenplusplus stephenplusplus changed the title Do not use catch-all without checking the instance of an error util.decorateRequest should confirm error is a missing project ID error Mar 14, 2017
@lukesneeringer lukesneeringer added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 19, 2017
@evaogbe evaogbe added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Aug 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

5 participants