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

Don't run step if miniprofiler doesn't exist #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oleg-polivannyi
Copy link

For some reason, miniprofiler object isn't accessible in middleware in some conditions

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 99.494% when pulling bf9403c on daniloisr:dont-run-step-if-miniprofiler-does-not-exist into fa9c21b on MiniProfiler:master.

@daniloisr
Copy link
Contributor

We tracked this bug and found that when using body-parser it mutates the req object and the miniprofiler getter becomes undefined (although we didn't find exactly where this happens).

This PR is a patch that don't affect most of timings, as conflict with body-parser only happens on request that uses req.body.

The proper solution for this problem is to refactor minprofiler internals to not depend on req object anymore, because using async_hooks we can access the current Nodejs context without using req, and I'm already working on this.

So this patch PR LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants