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

Having a function for lookup seems to only work for the first request #19

Open
kentmw opened this issue Sep 29, 2015 · 11 comments
Open

Comments

@kentmw
Copy link

kentmw commented Sep 29, 2015

I "consoled" the block about the lookup option being a function:

    if (typeof(opts.lookup) === 'function') {
      console.log('1:' + opts.lookup);
      middleware = function (middleware, req, res, next) {
        console.log('2:' + opts.lookup);
        return opts.lookup(req, res, opts, function () {
          console.log('3:' + opts.lookup);
          return middleware(req, res, next)
        })
      }.bind(this, middleware)
    }

Then I booted the server:

Tue Sep 29 2015 12:38:06 GMT-0400 (EDT): 1:function (req, res, opts, next) {
      if (req.body && req.body.uuid) {
        opts.lookup = ['body.uuid', 'params.uid'];
      } else {
        opts.lookup = ['connection.remoteAddress', 'params.uid'];
        opts.total = 100;
      }
      return next();
    } 

Then I ran the first request:

2:function (req, res, opts, next) {
      if (req.body && req.body.uuid) {
        opts.lookup = ['body.uuid', 'params.uid'];
      } else {
        opts.lookup = ['connection.remoteAddress', 'params.uid'];
        opts.total = 100;
      }
      return next();
    } 
3:connection.remoteAddress,params.uid 

Then I ran the second request:

2:connection.remoteAddress,params.uid 
Tue Sep 29 2015 12:38:23 GMT-0400 (EDT): TypeError: Property 'lookup' of object #<Object> is not a function

Notice the second go console log 2 has been updated to what was console log 3's.

@kentmw
Copy link
Author

kentmw commented Sep 29, 2015

I had to write a work around in my code:

 var limiterCreator = require('express-limiter')(app, client);
  function recreateLimiter(opts) {
    return function() {
      var limiter = limiterCreator(_.extend({}, opts));
      return limiter.apply(this, arguments);
    }
  }

Maybe I'm missing something really obvious but the fact that the limiter only gets created once and the original opts object is modified on subsequent calls seems like bad news. Perhaps cloning the opts at the beginning using underscore could mitigate this problem.

@ded
Copy link
Owner

ded commented Sep 29, 2015

i gotcha. i'll have a look to see what we can do about that

@vamonte
Copy link
Contributor

vamonte commented Dec 3, 2015

@kentmw , @ded : I had the same issue. I create a pull request to fix this issue. (#21)

@kentmw : Meanwhile the merge or another fix you may use my fork. ;)

@tuanquynet
Copy link

@vamonte: Your code worked.
@ded: Why don't you merge @vamonte's PR?

@vamonte
Copy link
Contributor

vamonte commented Dec 6, 2015

@ded Thank's for the merge. Do you know when you're going to tag the next release? I would like to use this package in an automated build.

@kiebzak
Copy link

kiebzak commented Jan 15, 2016

@vamonte ++

@cloud2pilot
Copy link

@vamonte ++
@ded just a reminder to tag the release.

@TheSoundDefense
Copy link

TheSoundDefense commented Jul 27, 2016

@vamonte ++
@ded We are also looking to use this package in an automated build in the near future, so a new tag would be much appreciated.

@v-kat
Copy link

v-kat commented Oct 7, 2016

New release please @ded

@nickdaugherty
Copy link

Just ran into this too, any word on a new release @ded?

@skeggse
Copy link

skeggse commented Nov 20, 2018

Is this fixed now, per #21?

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

10 participants