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

axios library throws error on createTopic #85

Closed
jamorales-bsft opened this issue Mar 3, 2018 · 12 comments
Closed

axios library throws error on createTopic #85

jamorales-bsft opened this issue Mar 3, 2018 · 12 comments
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@jamorales-bsft
Copy link

  • Environment details
    OS: GKE standard Google container OS
    Node.js version: 8.9.1
    npm version: 5.6.0
    google-cloud-node version: 0.16.4

  • Steps to reproduce
    Note: I ran it locally on my macbook it works however on GKE it fails with the following error when running the exact same code.

  1. require google-cloud
  2. Do PubSub.createTopic("a-Topic", aCallback)
  3. It throws the exception below. It seems to be from our logs the response of the authentication call from the google api that causes this error. From the stack trace it seems to be data passed to the axios library that fires the error

Get the following full stack trace
buffer.js:444
throw new TypeError(kConcatErrMsg);
^
TypeError: "list" argument must be an Array of Buffer or Uint8Array instances
at Function.Buffer.concat (buffer.js:444:13)
at IncomingMessage.handleStreamEnd (/var/components/live-meeting-session/node_modules/axios/lib/adapters/http.js:186:37)
at emitNone (events.js:111:20)
at IncomingMessage.emit (events.js:208:7)
at endReadableNT (_stream_readable.js:1056:12)
at _combinedTickCallback (internal/process/next_tick.js:138:11)
at process._tickCallback (internal/process/next_tick.js:180:9)
Logs from 3/3/18 4:13 AM to 3/3/18 4:13 AM UTC

@stephenplusplus
Copy link
Contributor

Hey @jamorales-bsft, thanks for reporting. I've tried this and haven't found it to fail for me. Could you show the minimum necessary code to reproduce? Are you authenticating with a key file or automatically through GKE?

Also, it could be a case where there was a temporary failure in a transient module. It might be worth re-deploying or otherwise updating the dependencies to see if something got sorted out on its own.

@stephenplusplus stephenplusplus added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Mar 5, 2018
@jamorales-bsft
Copy link
Author

@stephenplusplus Unfortunately this only happens when we deploy to k8. When run locally on my Mac and my coworkers Mac everything works fine. Also it only starts happening when using v0.16.x of the pubsub lib. 0.14.8 works fine with the exact same code. I included some code that reproduces it but again this only happens when deployed as a container on GKE. We have not tried minikube or docker directly. We also tried running it on linux locally to see that was breaking it and it worked so really not sure what it could be.

Here is the auth line we use
const gcloudPubsub = require("@google-cloud/pubsub")({keyFilename: global.gcloudAuthPath})

Here is the minimum code to reproduce on GKE

const gcloudPubsub = require("@google-cloud/pubsub")({keyFilename: global.gcloudAuthPath})

function pubsub (db, readyCB) {
	let topic = gcloudPubsub.topic("pubsubTopic")
	
	topic.get({ autoCreate: true }, createCB.bind(this, readyCB))
}

// arguments: readyCB, err, topic, apiResponse
function createCB (readyCB, err) {
	console.log("Never gets here, throws error first")
}```

@jamorales-bsft
Copy link
Author

Also we have redeployed this many times and also npm install from scratch so the dependencies should all be up to date

@stephenplusplus
Copy link
Contributor

Thanks for the extra information. I'll give it another shot.

@stephenplusplus
Copy link
Contributor

I re-deployed using the same code and auth style (a keyfile), but still didn't receive the error.

Looking at the stacktrace, it mentions: IncomingMessage.handleStreamEnd (node_modules/axios/lib/adapters/http.js:186:37. That file has evidently changed, since my environment is pulling a newer model, where the same line is now at 195 instead of 186. If you haven't already tried today, could you please run a fresh re-deploy to ensure you are on the latest dependencies?

@jamorales-bsft
Copy link
Author

jamorales-bsft commented Mar 5, 2018

I just tried again this time I:

1. deleted package-lock.json
2. npm rm --save @google-cloud/pubsub
3. rm -rf node_modules
4. npm install
5. npm install --save @google-cloud/pubsub
6. Then committed and ran another build. I got the same error on the same line 186

However it now reproduces locally and on GKE.

Here is a gist with a minimum package.json, package-lock.json and npm-list output. I do not know if that will help but maybe you can matchup what libraries you are getting with what I have.
https://gist.github.com/jamorales-bsft/3800a87e9f3818f1b8c24c470fe1ac3e

@stephenplusplus
Copy link
Contributor

Thanks for going through all of that. Does gcloudAuthPath point to a JSON service account key file?

@jamorales-bsft
Copy link
Author

yea it is of "type": "service_account"

@stephenplusplus
Copy link
Contributor

Could you see if you could get a longer stacktrace that shows where the error comes from within our library? Also, what are the details of your local environment?

- OS: 
- Node.js version: 
- npm version: 

@jamorales-bsft
Copy link
Author

I tried a bunch of things to get a longer stack trace and unfortunately nothing worked. I tried https://github.com/tlrobinson/long-stack-traces which was causing it's own crashes as well as setting the stack-size to 1000.

OS: Mac OS 10.13.3
node: v8.9.1
npm: 5.6.0

@JustinBeckwith JustinBeckwith added the 🚨 This issue needs some love. label Jun 8, 2018
stephenplusplus pushed a commit to stephenplusplus/nodejs-pubsub that referenced this issue Aug 31, 2018
@stephenplusplus
Copy link
Contributor

@jamorales-bsft since it's been so long, have you still been having this issue?

@jkwlui
Copy link
Member

jkwlui commented Dec 14, 2018

Closing this issue due to inactivity. Please reopen if new information comes up.

@jkwlui jkwlui closed this as completed Dec 14, 2018
@google-cloud-label-sync google-cloud-label-sync bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Jan 31, 2020
feywind pushed a commit to feywind/nodejs-pubsub that referenced this issue Nov 12, 2024
Fixes googleapis#85 

- [x] Tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

---

This PR should be ready to go. I believe this requires a major semver bump. I figured the test files shouldn't be updated in the same PR. The only thing that really needs to be pointed out is that classes shouldn't be instantiatable without the new keyword:

```js
const client1 = new Bigtable()
const client2 = Bigtable() // Error: Class constructor Foo cannot be invoked without 'new'
```

I was on the fence about requiring people to use the `new` keyword or not, and chose to make it optional. I did this by using a proxy:

```js
Bigtable = new Proxy(Bigtable, {
  apply(target, thisArg, argumentsList) {
    return new target(...argumentsList);
  },
});
```

Let me know if this should be removed. I didn't add that to the semi-private classes since the common case of creating one is something like `instance.table(...)` and not `new Table(...)`

Note - This was removed in review

As before, most of this was done via lebab so there may be some issues so please do go through it again to make sure nothing terrible is being done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. 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