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

parsing headers as integer cause failed to send usage report to apollo studio #7289

Closed
Blankll opened this issue Jan 8, 2023 · 2 comments
Closed

Comments

@Blankll
Copy link

Blankll commented Jan 8, 2023

Issue Description

when sending all headers to apollo studio

ApolloServerPluginUsageReporting({
      sendHeaders: { all: true },
      sendVariableValues: { all: true },
    }),

apollo server parse some of the request headers value to integer(I only observed field content-length parsed to an integer, not sure if other fields are also able to convert to non-string value),
headers below are the values we observed in code line during debugging

{
  "requestHeaders": {
    "host": {
      "value": [
        "localhost:4001"
      ]
    },
    "connection": {
      "value": [
        "keep-alive"
      ]
    },
    "content-length": {
      "value": [
        227
      ]
    }
  }
...
}

but usage report's proto-buf defined the header value as string

message Values map<string, Values> request_headers = 4;{
      repeated string value = 1;
    }
...
map<string, Values> request_headers = 4;

so we get the errors as bellow:

2023-01-08T15:19:27.472Z	xxxx 	ERROR	Error: Error encoding trace: http.requestHeaders.value: string[] expected
    at addTrace (/var/task/node_modules/@apollo/server/dist/cjs/plugin/usageReporting/plugin.js:395:35)
    at runNextTicks (node:internal/process/task_queues:61:5)
    at processImmediate (node:internal/timers:437:9)
    at process.callbackTrampoline (node:internal/async_hooks:130:17)

-- | --

Link to Reproduction

https://github.com/Blankll/serverless-event

Reproduction Steps

follow the repo's readme to package & deploy it to aws, checke the cloudwatch you will see the error message

@Blankll Blankll changed the title parse parsing content-length headers as integer cause failed to send usage report to apollo studio Jan 8, 2023
@Blankll Blankll changed the title parsing content-length headers as integer cause failed to send usage report to apollo studio parsing headers as integer cause failed to send usage report to apollo studio Jan 8, 2023
@glasser
Copy link
Member

glasser commented Jan 9, 2023

Is it possible to simplify your reproduction? Remove dependencies on things like newrelic and dynamodb, get it to be as small as possible and still reproduce the error? I'd rather not have to set up a New Relic account for an issue that presumably doesn't depend on New Relic. (And ideally if it can be reproduced locally without deploying to AWS that would be great to know as well!)

@glasser
Copy link
Member

glasser commented Jan 9, 2023

At a high level: the Apollo Server express middleware expects that the headers it receives should be strings or lists of strings.

I do see a few places where @vendia/serverless-express sets content-length headers to Buffer.byteLength return values, which are numbers. This seems wrong! I see some discussion at CodeGenieApp/serverless-express#563 about this causing problems. (Or well, that seems to be about express.static setting content-length to numbers, not serverless-express itself.) They suggest that serverless-express should stringify all headers but it hasn't been fixed.

I suggest opening a more explicit issue (or PR?) against @vendia/serverless-express adding stringification to the Buffer.byteLength calls?

I also see that this is fixed in serverless-adapter which is a more recent project that started as a fork of serverless-express. serverless-adapter actually has explicit built-in support for AS4 without even going through Express, so you might find that better than the vendia project. (It's definitely a newer and less widely used project though.)

@glasser glasser closed this as not planned Won't fix, can't repro, duplicate, stale Jan 9, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants