keymetrics / pm2-io-apm

PM2.io APM for Node.JS
Apache License 2.0
147 stars 40 forks source link

Swallowing Error instance included in context and replacing by {}. #270

Open BartoszRak opened 4 years ago

BartoszRak commented 4 years ago

Your package uses https://www.npmjs.com/package/@pm2/agent-node this one, so if we notifyError() that finally use WS agent to send our data out then WS agent make

  send (channel: string, payload: Object) {
    return this.agent.send(channel, this.getFormattedPayload(channel, payload)) ? 0 : -1
  }

where agent is:

const AgentNode = require('@pm2/agent-node')
...
...
...
this.agent = new AgentNode(this.config, this.process)

And if we dig into https://github.com/keymetrics/pm2-io-agent-node/blob/master/src/transport.js we find out that sending is handled by:

this.ws.send(JSON.stringify(packet))

Problem is that JSON.stringify() is not able to stringify Error instance, so If we put Error instance into our context we are going to see just {} which is annoying.

So I propose to maybe handle remapping part of context if its Error instance or something like that? But not sure it should be package or developer responsibility yet 🙆‍♂