Skip to content

Regression: @google-cloud/logging unhandled rejections during auth #2238

Closed
Closed
Regression: @google-cloud/logging unhandled rejections during auth#2238
Assignees
stephenplusplus
Labels
api: loggingIssues related to the Cloud Logging API.

Description

@ofrobots
ofrobots
opened on Apr 19, 2017

I wanted to try a performance benchmark now that 1.0.0 of @google-cloud/logging is now. The very basic example I started with seems to not work at all. Fails with unhandled rejections.

(node:44617) TypeError: Cannot read property 'config' of undefined
    at Auth.getAuthClient (/Users/ofrobots/tmp/log-t/node_modules/google-gax/node_modules/google-auto-auth/index.js:38:20)
    at /Users/ofrobots/tmp/log-t/node_modules/google-gax/lib/grpc.js:90:5
    at GrpcClient._getCredentials (/Users/ofrobots/tmp/log-t/node_modules/google-gax/lib/grpc.js:89:10)
    at GrpcClient.createStub (/Users/ofrobots/tmp/log-t/node_modules/google-gax/lib/grpc.js:172:15)
    at new LoggingServiceV2Client (/Users/ofrobots/tmp/log-t/node_modules/@google-cloud/logging/src/v2/logging_service_v2_client.js:122:38)
    at Object.LoggingServiceV2ClientBuilder.loggingServiceV2Client (/Users/ofrobots/tmp/log-t/node_modules/@google-cloud/logging/src/v2/logging_service_v2_client.js:832:12)
    at /Users/ofrobots/tmp/log-t/node_modules/@google-cloud/logging/src/index.js:603:52
    at Immediate.<anonymous> (/Users/ofrobots/tmp/log-t/node_modules/google-auto-auth/index.js:146:7)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)

A PR with the fix is here: googleapis/gax-nodejs#132.

This is a pretty big regression. How can we avoid such regressions going forward?

Activity

  1. bcomnes commented on Apr 19, 2017

    @bcomnes
    bcomnes
    on Apr 19, 2017

    Just wasted an hour on this to realize it was a deep transitive dependency regression. :( Thanks for the fix!

  2. jonsharratt commented on Apr 19, 2017

    @jonsharratt
    jonsharratt
    on Apr 19, 2017

    I have just hit this issue, life saver thanks for getting a fix in on this one, was in the same ⛵️ as @bcomnes :D Might be worth covering that issue with a test so it ensures it is all good going forward re: help stopping regressions.

  3. stephenplusplus commented on Apr 19, 2017

    @stephenplusplus
    stephenplusplus
    on Apr 19, 2017
    Contributor

    How can we avoid such regressions going forward?

    In terms of GCN, we would have to shrinkwrap our dependencies when releasing. Also, nightly cron jobs would catch these things. In terms of GAX, more tests :)

  4. stephenplusplus
    added
    api: loggingIssues related to the Cloud Logging API.
    on Apr 19, 2017

  5. stephenplusplus commented on Apr 19, 2017

    @stephenplusplus
    stephenplusplus
    on Apr 19, 2017
    Contributor

    Sorry to everyone who got burned by this. This is one of those things "that can happen sometimes" in npm, since we don't lock down our exact dependencies when publishing, and we instead rely on a semver-safe range. It's still within the realm of possibility that a package author will have an oversight and include a breaking change in a release that shouldn't have included one, or just a bug, like what happened in this case.

    I don't think shrinkwrapping is a road we want to go down, but we have an issue for nightly builds on the central tracker for GC* projects: https://github.com/GoogleCloudPlatform/gcloud-common/issues/222. That would catch things like this, where we could review any failed builds when they happen and look into the solution right away.

    For this bug, GCN doesn't have to do anything to include the fix once it's released in GAX. It will automatically be picked up in @google-cloud/logging installs. Considering that, I'm going to close this and let GAX address their testing procedure however they determine is right for their project, and the nightly build issue will hold the discussion around when & how to implement that.

  6. stephenplusplus
    closed this as completedon Apr 19, 2017

  7. bcomnes commented on Apr 19, 2017

    @bcomnes
    bcomnes
    on Apr 19, 2017

    FWIW yarn was able to keep my project working due to the lock file in this scenario. I ran into the issue with another dev using npm and was in a 'works in my env' type situation.

  8. ofrobots commented on Apr 19, 2017

    @ofrobots
    ofrobots
    on Apr 19, 2017
    ContributorAuthor

    @stephenplusplus Yes, I was also thinking something along the lines of a nightly build as well. I don't think we should shrinkwrap – that actually makes it harder to roll out fixes in a deep chain of dependencies. Users can and should use yarn to avoid such breaks at unexpected points.

    My concern was more around users finding bugs before we find them in a CI, and a nightly build should help here. I'll subscribe to that issue.

    BTW, FYI, [email protected] was released earlier today with the fix, so this should be fixed now.

  9. JustinBeckwith
    assigned
    stephenplusplus
    on Feb 1, 2021
  10. sofisl
    added a commit that references this issue on Feb 24, 2026

    feat: Support `fire-admin` tag for Admin Node SDK (#2238) (#2252)

    dde502f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: loggingIssues related to the Cloud Logging API.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions