devoidfury / express-debug

Debug toolbar middleware for developing applications in express (node.js)
Other
199 stars 16 forks source link

Doesn't play nicely with middleware that inject middleware #10

Open fzakaria opened 10 years ago

fzakaria commented 10 years ago

I'm using 'express-resource-new' which has the feature to define some middleware to be executed before each REST function.

I haven't dove too deep but when I add more than 1 middleware all hell breaks loose.

here is a stack:

stack: [ 'TypeError: Cannot read property \'EDT\' of undefined', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:103:31', ' at Array.forEach (native)', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:99:21', ' at Array.forEach (native)', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:96:16', ' at Array.forEach (native)', ' at Object.injections.route_profiler (/Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:93:29)', ' at Object.module.exports.request (/Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/index.js:29:16)', ' at Object.request (/Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/index.js:64:25)', ' at Object.EDT as handler' ], req:

fzakaria commented 10 years ago

Wanted to add a bit more information with links:

The package causing the problem: https://github.com/tpeden/express-resource-new

Specifically in this module you can specify handlers 'before' that are run before each REST route. The node debug tool oddly works when there is only one however when I have multiple it starts throwing the errors or the stack itself seems jumbled up.

fzakaria commented 10 years ago

Removing the profile middleware fixed the issue in the mean time .

fzakaria commented 10 years ago

Here's another stacktrace (this error doesn't occur when profile panel is missing):

[ 'TypeError: Cannot read property \'offset\' of undefined', ' at module.exports.index (/Users/zakariaf/Development/craterx/controllers/user/index.js:71:41)', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:45:18', ' at callbacks (/Users/zakariaf/Development/craterx/node_modules/express/lib/router/index.js:164:37)', ' at args.(anonymous function) (/Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:40:18)', ' at module.exports.ensureVerified (/Users/zakariaf/Development/craterx/utils/controller_helper.js:65:20)', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:45:18', ' at callbacks (/Users/zakariaf/Development/craterx/node_modules/express/lib/router/index.js:164:37)', ' at args.(anonymous function) (/Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:40:18)', ' at module.exports.ensureVerified (/Users/zakariaf/Development/craterx/utils/controller_helper.js:65:20)', ' at /Users/zakariaf/Development/craterx/node_modules/express-debug/lib/panels/profile/inject.js:45:18' ],

devoidfury commented 10 years ago

Thanks for the report!

Currently the profiler panel implementation wraps all the middleware functions (in app.stack) to handle function callback timing; an alternate implementation would be to replace app.router to time them instead. I've been wanting to switch to the alternate method for a while.

Edit: looks like app.handle is the way to go, but I'd like to find a way to do it without completely overwriting that core piece of functionality.