Bug in Rocky 3 : timeout setting in init() call not working

Libraries
#1

I noticed the Rocky framework doesn’t accept different timeout values from the default. When checking the code on github, I noticed the following :

    "init": function(settings = {}) {
    // Set defaults on a re-call
    _setDefaults();

    // Initialize settings, checking values as appropriate
    if ("timeout" in settings && typeof settings.timeout == "bool") _timeout = settings.timeout;

This looks like a bug. timeout is supposed to be an integer or a float in seconds, but it is only assigned to the field _timeout when type is a ‘bool’ (i.e. never…)

Looks like dangerous coding as well to put the check for key in a table together with using that very key in a logical if clause…I would normally do these checks sequentially, but that may be just me being overly cautious

#2

&& short-circuits, same as in C.

(oddly, though, I can’t find this stated in the documentation)

#3

ok, but i guess the typeof check for ‘bool’ is an error, right ?

#4

Definitely should not be a bool. I’ll PR a fix.

#5

Haven’t checked the lib code yet in detail, but I have the impression that also the per-route timeouts just default to 10 secs. No matter what we specifiy in the .post or .get call, it times out in 10 secs…
The assignment of timeout to the right class field in the method call seems ok, but is there maybe a ‘whichever triggers first’ timeout defined between global and per-route ?

It’s hitting us in some REST calls that need to collect a lot of async data.

#6

For completeness, I’m going to point out that agent-server imposes an upper limit of 10 minutes for HTTP timeouts. If your squirrel doesn’t call res.send(...) by then, we’ll return a 504 with “HTTP Request Handler Timeout”. There may also be another timeout imposed by the front proxy configuration, but I don’t recall one right now.

#7

I’ve not had this issue, at least up to 60s - see https://www.electricimp.com/secure-web-based-serial-terminal/ - but there I was using 2.0.2. Seems like the bug was introduced in 3.0.0.

#8

I don’t recall changing the underlying logic, just switching to a singleton table for 3.0.0, but I’ll have a look at the timeouts.