strongloop / strong-supervisor

Application supervisor that automatically adds cluster control and performance monitoring with StrongOps
Other
66 stars 20 forks source link

"Cannot read property 'duration' of undefined" when app receives request #213

Open kjdelisle opened 7 years ago

kjdelisle commented 7 years ago

Module versions: strong-supervisor@5.0.3 appmetrics@1.1.2 apiconnect-collective-member@1.4.10

Using the express-example-app@1.0.4 as my test application, after sending a GET request to the base route of the application, I receive this error:

pid:27864 worker:1 Listening on port: 9082
pid:27864 worker:1 TypeError: Cannot read property 'duration' of undefined
pid:27864 worker:1     at API.<anonymous> (/Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_modules/wlpn/nod
e_modules/wlpn-cli-server/node_modules/strong-supervisor/lib/adapter.js:381:30)
pid:27864 worker:1     at emitOne (events.js:77:13)

pid:27864 worker:1     at API.emit (events.js:169:7)
pid:27864 worker:1     at API.raiseLocalEvent (/Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_modules/wlpn
/node_modules/wlpn-cli-server/node_modules/strong-supervisor/node_modules/appmetrics/appmetrics-api.js:240:7)
pid:27864 worker:1     at Object.module.exports.emit (/Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_modul
es/wlpn/node_modules/wlpn-cli-server/node_modules/strong-supervisor/node_modules/appmetrics/index.js:231:18)
pid:27864 worker:1     at ExpressProbe.metricsEnd (/Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_modules/
wlpn/node_modules/wlpn-cli-server/node_modules/strong-supervisor/node_modules/appmetrics/probes/express-probe.js:89:6)
pid:27864 worker:1     at /Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_modules/wlpn/node_modules/wlpn-cl
i-server/node_modules/strong-supervisor/node_modules/appmetrics/probes/express-probe.js:61:18
pid:27864 worker:1     at args.(anonymous function) (/Users/kevin/.nvm/versions/node/v4.5.0/lib/node_modules/apiconnect-collective-member/node_module
s/wlpn/node_modules/wlpn-cli-server/node_modules/strong-supervisor/node_modules/appmetrics/lib/aspect.js:32:4)
pid:27864 worker:1     at Layer.handle [as handle_request] (/Users/kevin/wlpn/basic/package/node_modules/express/lib/router/layer.js:95:5)
pid:27864 worker:1     at next (/Users/kevin/wlpn/basic/package/node_modules/express/lib/router/route.js:131:13)
sam-github commented 7 years ago

@tobespc ----^ I haven't reproed, I'll try after looking at some other fires

sam-github commented 7 years ago

It looks like this may have been a bad packaging of apiconnect

sam-github commented 7 years ago

I reopened, because I think #216 is just a work-around, and that something unexpected is happening we need to figure out.

sjanuary commented 7 years ago

I don't see a 'response' in the express data emitted, am I missing something? https://github.com/RuntimeTools/appmetrics/blob/master/probes/express-probe.js#L82-L89 Perhaps this got rewritten at some point?

sjanuary commented 7 years ago

Yes, it looks like the express probe was refactored at some point. https://github.com/RuntimeTools/appmetrics/pull/260

As long as strong-supervisor is getting all the express data it needs from appmetrics I would say we can just delete that check

i.e. remove the whole of if (probeName === 'express'... and just have var duration = res.duration;

sam-github commented 7 years ago

supervisor is getting what it needs, but that isn't quite enough. If this is related to RuntimeTools/appmetrics#260, it probably means that the structure of the data is different when the user's app includes https://www.npmjs.com/package/strong-express-metrics, and perhaps that the probe name is wrong? strong-express-metrics is a bit odd, its something that the user manually adds as middleware, and which then sends extra information up the stack: appmetrics -> supervisor -> strong-pm -> strong-arc. Whether its there or not, supervisor should still work.