yahoo / node-limits Goto Github PK

When an incoming request is instrumented to add a timeout, the variable pulled from the config object to determine the timeout is config.incoming_timeout.

This conflicts with the global setup for timeouts - initialized using exposeGlobalTimers() where the variable that denotes this is called config.inc_req_timeout (which sets globalIncoming)

The former variable name is undocumented, and has caused unnecessary confusion.

This issue addresses this by retaining existing functionality for anyone who may be relying on it, but also preferring use of the documented variable config.inc_req_timeout in both cases.

We are seeing issues with limits not detaching the timer for inc_req_timeout. I think it may due to the free event not being called for incoming requests. I couldn't find any information on where this event is supposed to come from (I don't see it in stream or net API).

Should this instead be using the socket close event?

😄

https://github.com/yahoo/node-limits/blob/master/lib/index.js#L221

When you listen for data event on a stream, node immediately changes stream to flowing mode. All data will get dumped and if no one is catching it, the data is gone. Is there a reason we aren't wrapping req.emit and listening for the data event?

Only the end client should be the one to call req.on('data' ...