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

TypeError - req is required to have headers property #3286

Closed
1 of 3 tasks
Alastair-Spencer opened this issue Apr 26, 2023 · 4 comments · Fixed by #3289
Closed
1 of 3 tasks

TypeError - req is required to have headers property #3286

Alastair-Spencer opened this issue Apr 26, 2023 · 4 comments · Fixed by #3289
Assignees
Labels
agent-nodejs Make available for APM Agents project planning. bug community
Milestone

Comments

@Alastair-Spencer
Copy link

Alastair-Spencer commented Apr 26, 2023

Describe the bug

Since 3.42.0 and 3.44.0 a change has been made which now throws lambda runtimes with TypeErrors due to headers being missing utilised from a low level package basic-auth - https://github.com/jshttp/basic-auth/blob/v1.1.0/index.js#L80-L82. Areas around this package haven't been changed for a while so the cause of the issue is not quite clear yet although i'm trawling through changes in v3.42.0...v3.44.1 to try and help diagnose the cause.

Stack trace:

{
    "errorType": "TypeError",
    "errorMessage": "argument req is required to have headers property",
    "stack": [
        "TypeError: argument req is required to have headers property",
        "    at getAuthorization (/var/task/node_modules/basic-auth/index.js:88:11)",
        "    at auth (/var/task/node_modules/basic-auth/index.js:66:16)",
        "    at Object.getUserContextFromRequest (/var/task/node_modules/elastic-apm-node/lib/parsers.js:113:26)",
        "    at Immediate.<anonymous> (/var/task/node_modules/elastic-apm-node/lib/agent.js:525:24)",
        "    at processImmediate (node:internal/timers:466:21)",
        "    at process.callbackTrampoline (node:internal/async_hooks:130:17)"
    ]
}

To Reproduce

Steps to reproduce the behavior:

  1. Use this config:

Standard configuration applies from here on as the lambda is wrapped within the lambda handler and utilises environment configuration.

  ELASTIC_APM_SECRET_TOKEN: ${env:ELASTIC_APM_SECRET_TOKEN, ""}
  ELASTIC_APM_LOG_LEVEL: ${env:ELASTIC_APM_LOG_LEVEL, "info"}
  ELASTIC_APM_SERVER_URL: ${env:ELASTIC_APM_SERVER_URL, "http://localhost:8200"}
  ELASTIC_APM_LAMBDA_APM_SERVER: ${env:ELASTIC_APM_LAMBDA_APM_SERVER, ""}
  ELASTIC_APM_SERVICE_NAME: ${env:ELASTIC_APM_SERVICE_NAME, "${self:service}"}
  ELASTIC_APM_VERIFY_SERVER_CERT: true
  ELASTIC_APM_DATA_RECEIVER_TIMEOUT_SECONDS: 15
  1. Deploy and experience type errors from ALB traffic

Expected behavior

Environment (please complete the following information)

  • OS: Linux
  • Node.js version: 16
  • APM Server version: 3.44.0
  • Agent version: 3.44.0

How are you starting the agent? (please tick one of the boxes)

  • Calling agent.start() directly (e.g. require('elastic-apm-node').start(...))
  • Requiring elastic-apm-node/start from within the source code
  • Starting node with -r elastic-apm-node/start
@github-actions github-actions bot added agent-nodejs Make available for APM Agents project planning. community triage labels Apr 26, 2023
@trentm
Copy link
Member

trentm commented Apr 26, 2023

Thanks for the bug report!

At a guess, it will be one of these two commits:

commit 4444b9dbf1219f6e83f88f86569c0413b6134ddc
Date:   2023-02-02T10:12:18-08:00 (3 months ago)

    feat: support for Lambda function URL and ELB-triggered Lambdas (#3131)

    The transaction captured for a Lambda triggered by a Lambda function URL
    or by ELB (an Application Load Balancer targetting the Lambda function)
    will now include data according to spec
    (https://github.com/elastic/apm/blob/main/specs/agents/tracing-instrumentation-aws-lambda.md#elastic-load-balancer-elb).

    Closes: #2901

commit 3071cdf688d0b526c3f06ef6827204625890dd6a
Date:   2022-11-28T14:14:54-08:00 (5 months ago)

    feat: capture HTTP context for API Gateway-triggered Lambda functions (#3043)

    https://github.com/elastic/apm/blob/main/specs/agents/tracing-instrumentation-aws-lambda.md#api-gateway--lambda-urls
    > The agent should use the information in the request and response
    > objects to fill the HTTP context (context.request and context.response)
    > fields in the same way it is done for HTTP transactions.

    Closes: #2419

Both of those commits added a trans.req = { ... } object that is meant to mimic a Node.js http.IncomingMessage object. If trans.req is set, then serialization of the APM transaction object calls parsers.getUserContextFromRequest(this.req) (here) and that function uses the basic-auth module.

Here is the release and commit order for those commits:

* 64b2ffc2 - (tag: v3.43.0) 3.43.0 (#3184) (8 weeks ago)
...
* 4444b9db - feat: support for Lambda function URL and ELB-triggered Lambdas (#3131) (3 months ago) 
...
* ec046ad6 - (tag: v3.42.0) 3.42.0 (#3103) (3 months ago)
...
* 89b432f8 - (tag: v3.41.1) 3.41.1 (#3077) (4 months ago)
...
* bd28207b - (tag: v3.41.0) 3.41.0 (#3064) (5 months ago)
...
* 3071cdf6 - feat: capture HTTP context for API Gateway-triggered Lambda functions (#3043) (5 months ago)

Given you said the issue is in v3.42.0, I'm guessing this is with API Gateway trigger usage, and that the problem also exists in v3.41.x.


TIL I learned that basicAuth() can throw. Its README for that function suggests it would return undefined:

Get the basic auth credentials from the given request. The `Authorization`
header is parsed and if the header is invalid, `undefined` is returned,
otherwise an object with `name` and `pass` properties.
> var basicAuth = require('basic-auth')
undefined
> basicAuth({})
Uncaught TypeError: argument req is required to have headers property
    at getAuthorization (/Users/trentm/el/apm-agent-nodejs/node_modules/basic-auth/index.js:88:11)
    at auth (/Users/trentm/el/apm-agent-nodejs/node_modules/basic-auth/index.js:66:16)
> basicAuth({headers: null})
Uncaught TypeError: argument req is required to have headers property
    at getAuthorization (/Users/trentm/el/apm-agent-nodejs/node_modules/basic-auth/index.js:88:11)
    at auth (/Users/trentm/el/apm-agent-nodejs/node_modules/basic-auth/index.js:66:16)
> basicAuth({headers: {}})
undefined

It will be straightforward to guard on this.


Can you confirm that you are using a Lambda function that is triggered via API Gateway or ELB (ALB)?

Apparently the event to the Lambda function can exclude a event.headers field, at least in your case.
If you are able, I would be curious to see what the output of this at the top of your Lambda function is:

console.log('event keys:', Object.keys(event))
console.log('event.headers:', event.headers)

If my guess is correct, as a workaround until we have a fix, you could switch to v3.40.0 or you could add this to the top of your handler function(s):

event.headers = event.headers || {}

@trentm trentm self-assigned this Apr 26, 2023
@trentm trentm added this to the 8.9 milestone Apr 26, 2023
@trentm trentm removed the triage label Apr 26, 2023
@trentm trentm added this to Planned in APM-Agents (OLD) via automation Apr 26, 2023
@trentm trentm moved this from Planned to In Progress in APM-Agents (OLD) Apr 26, 2023
@trentm
Copy link
Member

trentm commented Apr 26, 2023

I can reproduce if I manually remove the event.headers field before the APM agent does its processing, resulting in this in the invocation log:

{"log.level":"debug","@timestamp":"2023-04-26T22:36:28.769Z","log":{"logger":"elastic-apm-node"},"ecs":{"version":"1.6.0"},"error":{"type":"TypeError","message":"argument req is required to have headers property","stack_trace":"TypeError: argument req is required to have headers property
    at getAuthorization (/opt/nodejs/node_modules/elastic-apm-node/node_modules/basic-auth/index.js:88:11)
    at auth (/opt/nodejs/node_modules/elastic-apm-node/node_modules/basic-auth/index.js:66:16)
    at Object.getUserContextFromRequest (/opt/nodejs/node_modules/elastic-apm-node/lib/parsers.js:113:26)
    at Immediate.<anonymous> (/opt/nodejs/node_modules/elastic-apm-node/lib/agent.js:525:24)
    at process.processImmediate (node:internal/timers:471:21)
    at process.callbackTrampoline (node:internal/async_hooks:130:17)"},"message":"Elastic APM caught unhandled exception"}

trentm added a commit that referenced this issue Apr 27, 2023
…eaders'

If the lambda handler object for an ELB- or API Gateway-triggered Lambda
invocation did not have a 'headers' field, then the Lambda
instrumentation would crash. I'm not sure how to get an event with no
'headers' field, but we should still be defensive here.

Fixes: #3286
APM-Agents (OLD) automation moved this from In Progress to Done Apr 27, 2023
trentm added a commit that referenced this issue Apr 27, 2023
…eaders' (#3289)

If the lambda handler object for an ELB- or API Gateway-triggered Lambda
invocation did not have a 'headers' field, then the Lambda
instrumentation would crash. I'm not sure how to get an event with no
'headers' field, but we should still be defensive here.

Fixes: #3286
@trentm
Copy link
Member

trentm commented Apr 27, 2023

@Alastair-Spencer We have merged a fix. I will try to get a release out soon.

If you are able, I am still very curious to know how you got an event object to your Lambda function handler that did not have a event.headers field.

@trentm trentm mentioned this issue Apr 27, 2023
@trentm trentm added the bug label Apr 27, 2023
@Alastair-Spencer
Copy link
Author

Hi @trentm 👋🏻

Thanks for the reply and no worries on the bug report.

Can you confirm that you are using a Lambda function that is triggered via API Gateway or ELB (ALB)?

Our lambdas are invoked using ELB (ALB) and the 3.42.0 is the stable version we're using currently.

I'll try and get you some logs on a deployed version which can help 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. bug community
Projects
2 participants