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

Failed to "modifyAckDeadline": Metadata key contains illegal characters #680

Closed
katmatt opened this issue Jul 1, 2019 · 14 comments
Closed
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. needs more info This issue needs more information from the customer to proceed. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@katmatt
Copy link

katmatt commented Jul 1, 2019

Environment details

  • OS: linux
  • Node.js version: 10.15.3
  • yarn version: 1.15.2
  • @google-cloud/pubsub version: 0.30.1

Steps to reproduce

We use code similar to the sample

function listenForMessages(subscriptionName, timeout) {
to receive messages, but use the following subscription config:

    const options = {
      flowControl: {
        allowExcessMessages: false,
        maxMessages: 20,
        maxExtension: 10,
      },
    }
    pubSub.subscription(subscriptionName, options)

We see the following error messages in our logs:

Error: Failed to "modifyAckDeadline" for 9 message(s). Reason: Metadata key "a" contains illegal characters
    at Object.keys.map (/base/node_modules/@google-cloud/pubsub/src/message-queues.ts:238:15)

We haven't seen this message before we updated to version 0.30.0 and this error is still reported after updating to version 0.30.1.

@callmehiphop callmehiphop added the status: duplicate Duplicate. label Jul 1, 2019
@callmehiphop
Copy link
Contributor

Going to close this out as a duplicate to #667. I would suggest pinning to 0.29.0 for a temporary workaround.

@katmatt
Copy link
Author

katmatt commented Jul 1, 2019

Now both issues are closed... But I guess you keep these issues tracked on an internal issue tracker? Just curious what your plan is to solve this issue 😄

@callmehiphop
Copy link
Contributor

@katmatt ahh, no, if you read through #667 you would come across googleapis/nodejs-datastore#415 which is the tracking issue. The bug is something we're seeing all over and isn't something for the client itself to fix.

@callmehiphop
Copy link
Contributor

Actually, going down the rabbit hole this was marked as resolved. @katmatt do you know what version of google-gax is installed? Looks like >= 1.1.2 should have done the trick.

@katmatt
Copy link
Author

katmatt commented Jul 1, 2019

According to our yarn.lockfile we are using version google-gax "^1.0.0". But we can try to replace it with the version you mentioned 😄

@katmatt
Copy link
Author

katmatt commented Jul 1, 2019

And this version is pulled in from @google-cloud/pubsub@^0.30.0:

"@google-cloud/pubsub@^0.30.0":
  version "0.30.0"
  dependencies:
    "@google-cloud/paginator" "^1.0.0"
    "@google-cloud/precise-date" "^1.0.0"
    "@google-cloud/projectify" "^1.0.0"
    "@google-cloud/promisify" "^1.0.0"
    "@sindresorhus/is" "^0.17.1"
    "@types/duplexify" "^3.6.0"
    "@types/long" "^4.0.0"
    arrify "^2.0.0"
    async-each "^1.0.1"
    extend "^3.0.2"
    google-auth-library "^3.0.0"
    google-gax "^1.0.0"
    is-stream-ended "^0.1.4"
    lodash.snakecase "^4.1.1"
    p-defer "^3.0.0"
    protobufjs "^6.8.1"

Which corresponds to

"google-gax": "^1.0.0",
😉

@callmehiphop
Copy link
Contributor

@katmatt sorry for the late response but the ^ symbol should pull in any minor/patch changes automatically. I'm guessing your lock file might have prevented that from happening. Did deleting the lock file and reinstalling work for you?

@katmatt
Copy link
Author

katmatt commented Jul 10, 2019

@callmehiphop Thanks for the follow up, but updating our yarn.lock didn't fix this issue. We now use version 1.1.4 of google-gax and still see the following errors in our logs:

Error: Failed to "acknowledge" for 1 message(s). Reason: Metadata key " 'q¾" contains illegal characters
    at AckQueue._sendBatch (/base/node_modules/@google-cloud/pubsub/src/message-queues.ts:194:13)

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Jul 10, 2019
@bcoe
Copy link
Contributor

bcoe commented Jul 15, 2019

@katmatt it sounds like 1.1.5 should potentially address the invalid character issue, mind updating your dependency accordingly, and taking it for a spin?

@bcoe bcoe added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. needs more info This issue needs more information from the customer to proceed. and removed 🚨 This issue needs some love. status: duplicate Duplicate. triage me I really want to be triaged. labels Jul 15, 2019
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Jul 15, 2019
@katmatt
Copy link
Author

katmatt commented Jul 18, 2019

@bcoe We updated to version 1.1.5 of google-gax and it didn't fix this issue. We now see a different, but somehow related error in our logs:

(node:1) Error: Failed to add metadata entry  §NÜ: Thu, 18 Jul 2019 10:39:54 GMT. Metadata key " §nü" contains illegal characters

@bcoe
Copy link
Contributor

bcoe commented Jul 19, 2019

CC: @alexander-fenster ☝️ is it 1.1.5 that we should be pointing folks towards?

@alexander-fenster
Copy link
Contributor

Yeah, and it behaves as expected. We did not fix the issue, we just made it so it does not crash, because the issue itself is beyond our control.

@katmatt To give you some details: the error is actually coming from Node.js http2 stack. We have an issue filed in Node.js repository, but given that (a) the error is hard to repro, and (b) it's some encrypted bytes so we can't just tcpdump and see what's going on, it's likely to take some time to investigate and fix. I wouldn't expect this to be fixed in Node.js tomorrow.

So, to make this work, we decided we'd just warn users if an invalid header is received, but we won't crash. These are the lines in @grpc/grpc-js that are printing the warning you see. google-gax 1.1.5 depends on this 'fixed' version of @grpc/grpc-js.

Long story short: your code won't crash anymore, but it will still bug you until the upstream Node.js problem is fixed.

@katmatt
Copy link
Author

katmatt commented Jul 26, 2019

@alexander-fenster I see and that means that we can ignore the logged message. Thanks a lot for your help!

@bcoe
Copy link
Contributor

bcoe commented Jul 29, 2019

@katmatt 👍 awesome. Going to go ahead and close this issue, please let us know if you bump into any related issues.

@bcoe bcoe closed this as completed Jul 29, 2019
@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
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. needs more info This issue needs more information from the customer to proceed. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. triage me I really want to be triaged. 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

6 participants