aws / aws/aws-xray-sdk-node

http client error events might be suppressed

Open
#318 3 comments 0 reactions 1 assignee Claimed by @willarmiros View on GitHub
bug
Dominant language
JavaScript
Stars
280
Forks
157
PR merge metrics
No merged PRs in 30d

Description

I think [this check](https://github.com/aws/aws-xray-sdk-node/blob/0590805fe2591ba7cc2c6891caec4ef43fed40e1/packages/core/lib/patchers/http_p.js#L141) in http_p.js should be improved as it may result in not re-emitting an error event.

`this._events.error` is either an Array (if more then 1 listener is installed) or `Function` (only 1 listener installed).
Currently this check works because the installed listener `errorCapturer` has exactly 1 named parameter therefore `errorCapturer.length === 1`.
But if there is a module used which wraps the installed listeners (e.g. for CLS,...) or if `once` is used to install the listener this is broken. Besides that it relies on internals of `EventEmitter` (the `_events` field`).

I recommend to use the public `EventEmitter` Api to check that <= 1 listener is installed.

Recent NodeJs versions allow to listen on [errorMonitor](https://nodejs.org/dist/latest-v12.x/docs/api/events.html#events_eventemitter_errormonitor) to avoid the need to re-emit in case only 1 listener is present at all.

Edit:
Found that also [postgres](https://github.com/aws/aws-xray-sdk-node/blob/9cb04b6ddcabca33dc483c8e1d3e82d6fb108e80/packages/postgres/lib/postgres_p.js#L109) and [mysql](https://github.com/aws/aws-xray-sdk-node/blob/9cb04b6ddcabca33dc483c8e1d3e82d6fb108e80/packages/mysql/lib/mysql_p.js#L209) is effected.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.