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

Setting requestWhitelist doesn't seem to work #92

Closed
slooker opened this issue Oct 21, 2015 · 9 comments
Closed

Setting requestWhitelist doesn't seem to work #92

slooker opened this issue Oct 21, 2015 · 9 comments

Comments

@slooker
Copy link

slooker commented Oct 21, 2015

I can add to requestWhitelist like the docs suggest, but if I remove from requestWhitelist, it doesn't seem to work.

I've tried setting the white list like so:

expressWinston.requestWhitelist = [
  "url"
];

But the whitelist on line 216 of index.js still has the full requestWhitelist, so I'm still getting all of the data.

@satyadeepk
Copy link

I am facing the same issue. Have you found any workaround?

@floatingLomas
Copy link
Collaborator

That makes no sense. But you're correct. The test suite also doesn't test for being able to edit/change this.

I'll try to look at this at some point, or you're more than welcome to fork and submit a pull request.

@aggied
Copy link

aggied commented Oct 28, 2015

same problem for me. Am hoping to just be able to set the array and be done w/ it.

@rosston
Copy link
Collaborator

rosston commented Feb 25, 2016

There is a workaround. The trick is that you always need to modify the existing requestWhitelist array, without actually reassigning the variable.

Here's an example:

// what you want to do
expressWinston.requestWhitelist = ["url", "foo"];

// what will actually work
expressWinston.requestWhitelist.splice(0);
expressWinston.requestWhitelist.push.apply(expressWinston.requestWhitelist, ["url", "foo"]);

Yes, this is a dirty hack. I'm working on fixing this properly, and I should have it out in a day or two.

@trvsdnn
Copy link

trvsdnn commented Feb 25, 2016

Any reason this can't just be passed into the constructor with all of the other options?

@rosston
Copy link
Collaborator

rosston commented Feb 26, 2016

No, the whitelists and blacklists cannot simply be options to pass to the middleware factory because there's more than one middleware factory, both of which I would expect are typically used together in the same app. And presumably the express-winston user wants the whitelists and blacklists to be consistent across their regular logs and error logs.

@nwah
Copy link
Contributor

nwah commented Mar 1, 2016

One way to make it a little more comfortable to replace the whitelist would be something like this:

var expressWinston = require('express-winston');
expressWinston.config.requestWhitelist = [...];
expressWinston.config.ignoredRoutes = [...];

Or

var expressWinston = require('express-winston');
expressWinston.whitelists.request = [...];
expressWinston.blacklists.body = [...];
expressWinston.ignored.routes = [...];

By exposing them under an intermediate object/property, people could replace them in a more natural way. Shouldn't be too hard to implement under the hood either, and could also easily be implemented to be backward compatible with the current implementation.

@rosston
Copy link
Collaborator

rosston commented Mar 1, 2016

@nwah I planned on simply making the *Whitelist and *Blacklist exported properties assignable (as the OP expected) rather than needing to keep the reference intact. I feel like simple assignment to the module properties is the naturally expected behavior given the documentation in the Readme around push-ing to those arrays.

Your config object suggestion does seem a little more consistent with how other node modules work, but I would still lean toward simple assignment for backward compatibility/API consistency. However, I could be convinced otherwise. Perhaps I'm missing something?

rosston pushed a commit that referenced this issue Mar 9, 2016
Fixes #92.

Allows user to override module-level whitelists and functions without
having to worry about using the right object reference.
@rosston
Copy link
Collaborator

rosston commented Mar 9, 2016

Published in v1.3.0.

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

No branches or pull requests

7 participants