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 due to missing sanitizeFieldNamesRegExp #3247

Closed
d0ruk opened this issue Apr 6, 2023 · 3 comments · Fixed by #3249
Closed

TypeError due to missing sanitizeFieldNamesRegExp #3247

d0ruk opened this issue Apr 6, 2023 · 3 comments · Fixed by #3249
Assignees
Labels
agent-nodejs Make available for APM Agents project planning. bug community

Comments

@d0ruk
Copy link

d0ruk commented Apr 6, 2023

Hello,

I am initialising the APM client at the top of my main module like so

const config = require('./config');
const { log } = require('./helpers');

if (config.apm.enabled) {
  require('elastic-apm-node').start({
    serviceName: config.apm.serviceName,
    secretToken: config.apm.secretToken,
    serverUrl: config.apm.serverUrl,
    serviceVersion: config.version,
    environment: config.apm.environment,
    captureBody: 'errors',
    logUncaughtExceptions: false,
  });
}

However, app crashes with a TypeError: Cannot read property 'push' of undefined referring to sanitizeFieldNamesRegExp.

I do not specify sanitizeFieldNames or sanitizeFieldNamesRegExp in the options, as seen above. The library then uses the default values which are initialized as arrays.

Do you have any idea as to why this happens?


The require('./config') that come before has

require('dotenv').config();
const env = { ...process.env };

Can this be where the problem is?

I'd appreciate any help/comments on this issue.

Note: I am using elastic-apm-node ^3.42.0 on Node.js 14.

Best,
Doruk

@github-actions github-actions bot added agent-nodejs Make available for APM Agents project planning. community triage labels Apr 6, 2023
@trentm
Copy link
Member

trentm commented Apr 6, 2023

@d0ruk Thanks for the issue! I'll start trying to reproduce this.

Are you able to post the full traceback? Perhaps add the logUncaughtExceptions: true config var to the .start(...) options because the APM agent doesn't print uncaught exception tracebacks by default (it will in the next major version).

@trentm trentm added bug and removed triage labels Apr 6, 2023
@trentm trentm self-assigned this Apr 6, 2023
@trentm trentm added this to Planned in APM-Agents (OLD) via automation Apr 6, 2023
@trentm trentm moved this from Planned to In Progress in APM-Agents (OLD) Apr 6, 2023
@d0ruk
Copy link
Author

d0ruk commented Apr 6, 2023

@trentm

Here is the stack.


TypeError: Cannot read property 'push' of undefined
  File "/home/node/node_modules/elastic-apm-node/lib/config.js", line 902, col 37, in normalizeSanitizeFieldNames
    opts.sanitizeFieldNamesRegExp.push(re)
  File "/home/node/node_modules/elastic-apm-node/lib/config.js", line 735, col 3, in normalize
    normalizeSanitizeFieldNames(opts)
  File "/home/node/node_modules/elastic-apm-node/lib/config.js", line 498, col 13, in Client.<anonymous>
    normalize(conf, agent.logger)
  File "events.js", line 400, col 28, in Client.emit
  File "domain.js", line 475, col 12, in Client.emit
  File "/home/node/node_modules/elastic-apm-http-client/index.js", line 427, col 14, in <anonymous>
    this.emit('config', config)
  File "/home/node/node_modules/fast-stream-to-buffer/index.js", line 20, col 9, in IncomingMessage.<anonymous>
    cb(err, buffers[0], stream)
  File "/home/node/node_modules/once/once.js", line 25, col 25, in IncomingMessage.f
    return f.value = fn.apply(this, arguments)
  File "/home/node/node_modules/end-of-stream/index.js", line 36, col 27, in IncomingMessage.onend
    if (!writable) callback.call(stream);
  File "events.js", line 412, col 35, in IncomingMessage.emit
  File "domain.js", line 475, col 12, in IncomingMessage.emit
  File "internal/streams/readable.js", line 1333, col 12, in endReadableNT
  File "internal/process/task_queues.js", line 82, col 21, in processTicksAndRejections

This error was captured on Node.js 14.19.3 with elastic-apm-http-client 11.2.0 & elastic-apm-node 3.43.0.

@trentm
Copy link
Member

trentm commented Apr 6, 2023

@d0ruk

That is a code path coming from Central config. Thanks very much, that traceback is helpful! I can reproduce the error and will work on a fix now.

I suspect you have APM Agent central config setup in your Kibana to configure sanitize_field_names for this or all services. Can you confirm that?
If you run your app with trace-level logging (set ELASTIC_APM_LOG_LEVEL=trace or logLevel: 'trace') and re-run, then you should see a log line something like:

{"log.level":"debug","@timestamp":"2023-04-06T16:29:26.664Z","log":{"logger":"elastic-apm-node"},"remoteConf":{"sanitize_field_names":"password, pass*"},"ecs":{"version":"1.6.0"},"message":"central config received"}

As a workaround, if you need it while I get a fix out, you could remove "sanitize_field_names" from your central configuration in Kibana.

trentm added a commit that referenced this issue Apr 6, 2023
The crash was with init of config.sanitizeFieldNamesRegExp in the central-config
code path. I've fixed that. Other updates:

- Updated config normalization of this var -- and similar config vars
  that set a `${name}RegExp` array var -- initialize the RegExp array to empty
  before populating it.
- Updated the tests to ensure we always add to "central-config-enabled.test.js"
  whenever a new supported central config var is added.
- Updated central-config handling to try/catch and log.error instead of
  *crashing* if there is an unexpected exception. Partially applied central
  config isn't great, but it is better than having a DoS avenue where central
  config can crash agents. For example:
    {"log.level":"error",...,"remoteConf":{"sanitize_field_names":"password, pass*"},"error":{"type":"TypeError","message":"Cannot read properties of undefined (reading 'push')","stack_trace":"..."},"message":"Central config error: exception while applying changes"}

Fixes: #3247
APM-Agents (OLD) automation moved this from In Progress to Done Apr 6, 2023
trentm added a commit that referenced this issue Apr 6, 2023
…#3249)

The crash was with init of config.sanitizeFieldNamesRegExp in the central-config
code path. I've fixed that. Other updates:

- Updated config normalization of this var -- and similar config vars
  that set a `${name}RegExp` array var -- initialize the RegExp array to empty
  before populating it.
- Updated the tests to ensure we always add to "central-config-enabled.test.js"
  whenever a new supported central config var is added.
- Updated central-config handling to try/catch and log.error instead of
  *crashing* if there is an unexpected exception. Partially applied central
  config isn't great, but it is better than having a DoS avenue where central
  config can crash agents. For example:
    {"log.level":"error",...,"remoteConf":{"sanitize_field_names":"password, pass*"},"error":{"type":"TypeError","message":"Cannot read properties of undefined (reading 'push')","stack_trace":"..."},"message":"Central config error: exception while applying changes"}

Fixes: #3247
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
Development

Successfully merging a pull request may close this issue.

2 participants