Error-circular - where is this coming from?

I'm getting "error-circular" from time to time, but there's no ID or source shown.

I can't find a pattern to when it happens. I'm running Node-RED as a system service so how would I see more detailed errors to determine the source? Sorry if the answer to this is obvious.

try adding a catch node.
Click on the all nodes and change it to current flow to see if you can locate which flow tab is is coming from

Thanks, bizarrely the catch node didn't output anything, and I cycled through all tabs with "current flow" selected but they were all blank, until I switched back to "All nodes"

But I think I've found the cause, it's coming from a function. Not sure why it's doing it, but I can at least try and figure out how to stop it!

I think the error-circular is coming from the function within a function. Could it be?

Is there a better way to achieve the following: a timeout within a timeout, to sequence two things one after the other?

var id = setTimeout(function(){
    var i = startval;
    function f() {
        if (i>0) {
            i--;
            msg.payload = i;
            node.send(msg);
        } else {
            node.status({});
            return;
        }
        id = setTimeout( f, steptime );
    }
    f();
},initialdelay)

Could you post all the code of the function node or the flow containing the function node.

Hi @cflurin it's the same one I was working on before:

[{"id":"de21cbd7.c9a8c8","type":"function","z":"fa3892e2.c6cfd","name":"Fade Out","func":"// Check for correct incoming values\nif (!msg.reset &&\n    (!msg.payload ||\n     !msg.dimtime ||\n     !msg.initialdelay)) {\n    node.warn(\"Fader not correctly configured\");\n    return;\n}\n\nvar thisnodeid = \"fader_\" + node.id;\n\n// Get incoming values\nvar startval = Number(msg.payload); // just in case incoming was a string\nvar dimtime = msg.dimtime * 1000;\nvar initialdelay = msg.initialdelay * 1000;\nvar steptime = dimtime / startval;\n\n// If we got a msg.reset, cancel timer, clear status,\n//  clear function from memory, and don't restart\nif (msg.reset) {\n    var id = context.get(thisnodeid);\n    clearTimeout(id);\n    node.status({});\n    context.set(thisnodeid,\"\");\n    return;\n}\n\n// if context.get('id') has an object, then it's running\n// and if it's running then clear the timer, clear the memory,\n//  then continue (i.e. start over with new fader)\nif (context.get('id') !== '') {\n    var id = context.get(thisnodeid);\n    clearTimeout(id);\n    node.status({});\n    context.set(thisnodeid,\"\");\n}\n\n// Set initial light level\nmsg.payload = startval; // (use startval in case input was string)\nnode.send(msg);\n\nvar id = setTimeout(function(){\n    var i = startval;\n    function f() {\n        if (i>0) {\n            i--;\n            msg.payload = i;\n            node.send(msg);\n            var fadetxt = \"Fading \" + (i*steptime/1000).toString() + \"s, \" + i.toString() + \"/\" + startval.toString();\n            node.status({fill:\"green\", shape:\"ring\", text:fadetxt});\n        } else {\n            node.status({});\n            context.set(thisnodeid,\"\");\n            return;\n        }\n        id = setTimeout( f, steptime );\n        context.set(thisnodeid,id);\n    }\n    f();\n},initialdelay)\ncontext.set(thisnodeid,id);\n\nnode.status({fill:\"blue\", shape:\"ring\", text:\"Delay \" + msg.initialdelay.toString() + \"s\"});","outputs":1,"noerr":0,"x":520,"y":1800,"wires":[["74498acb.008e74"]]},{"id":"2817b09c.9bb97","type":"inject","z":"fa3892e2.c6cfd","name":"","topic":"","payload":"","payloadType":"date","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":200,"y":1800,"wires":[["36c2255d.c5dbda"]]},{"id":"36c2255d.c5dbda","type":"change","z":"fa3892e2.c6cfd","name":"Fader Settings","rules":[{"t":"set","p":"payload","pt":"msg","to":"10","tot":"str"},{"t":"set","p":"initialdelay","pt":"msg","to":"3","tot":"str"},{"t":"set","p":"dimtime","pt":"msg","to":"7","tot":"str"}],"action":"","property":"","from":"","to":"","reg":false,"x":360,"y":1800,"wires":[["de21cbd7.c9a8c8"]]},{"id":"74498acb.008e74","type":"debug","z":"fa3892e2.c6cfd","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","x":670,"y":1800,"wires":[]}] 

The error happens randomly during running the function. Sometimes it doesn't happen at all. Never at the same time in the process.

It seems that you can't save the id of the setTimeout() in a context variable using context.set().

Probably @dceejay or @knolleary can explain the reason.

Try this:

// Check for correct incoming values
if (!msg.reset &&
    (!msg.payload ||
     !msg.dimtime ||
     !msg.initialdelay)) {
    node.warn("Fader not correctly configured");
    return;
}

// Get incoming values
var startval = Number(msg.payload); // just in case incoming was a string
var dimtime = msg.dimtime * 1000;
var initialdelay = msg.initialdelay * 1000;
var steptime = dimtime / startval;

// If we got a msg.reset, cancel timer, clear status,
//  clear function from memory, and don't restart
if (msg.reset) {
    clearTimeout(context.id);
    node.status({});
    return;
}

// if context.id has an object, then it's running
// and if it's running then clear the timer, clear the memory,
//  then continue (i.e. start over with new fader)
if (context.id) {
    clearTimeout(context.id);
    node.status({});
}

// Set initial light level
msg.payload = startval; // (use startval in case input was string)
node.send(msg);

context.id = setTimeout(function(){
    var i = startval;
    function f() {
        if (i>0) {
            i--;
            msg.payload = i;
            node.send(msg);
            var fadetxt = "Fading " + (i*steptime/1000).toString() + "s, " + i.toString() + "/" + startval.toString();
            node.status({fill:"green", shape:"ring", text:fadetxt});
        } else {
            node.status({});
            return;
        }
        context.id = setTimeout( f, steptime );
    }
    f();
},initialdelay)

node.status({fill:"blue", shape:"ring", text:"Delay " + msg.initialdelay.toString() + "s"});

I don't know why it should cause that error, but a couple of points.
A variable declared using var is (I believe) effectively global to the node, redefining it again with var does not make it a new variable, therefore all your id variables are the same variable. If they are supposed to be different variables then give them different names. Note that in the editor the lines are marked with a yellow triangle to indicate there is a problem. If you must call them all the same name then use let rather than var.
It is a good idea to check that an id is valid before calling clearTimeout. I suggest setting the context variable to null when setting it rather than "", then you can do

if (id) clearTimeout(id);

Don't use return inside a timeout function, there is nothing to return from. I don't know whether it will cause problems or not. You can remove the return and move the

        id = setTimeout( f, steptime );
        context.set(thisnodeid,id);

to inside the if statement, assuming that is what you want.

I am fairly sure that is not correct, I am sure I have done it.

Did you set ContextStorage in settings.js ?

I have not used it with persistent storage if that is what you mean.
Certainly there would be no point saving a timer id in persistent storage as the timer itself would not survive the restart anyway.

Well, there is a difference when you set contextStorage (persistent storage)

    contextStorage: {
        default: {
	          module:"localfilesystem"
        },

Sure, you don't need persistence for the id but if you just use context.set() and contextStorage than the id is also stored in a file. That's the reason I proposed to use context.id = setTimeout().

Anyway I can reproduce the error-cirular with context.set() plus contextStorage.

OK, that makes sense (or at least I can see how it might make sense). For persistent storage node-RED has to serialise the object being stored in order to write it out, and I believe it is the serialisation of circular objects that causes the error.
If that is the case then it is not the fact that context.set() cannot be used, but it cannot be used with a storage method that requires serialisation, such as file storage. Hopefully it will be ok if you select memory store.

or just use context.myname like I suggested originally in the other thread...
BUT you need to get rid of that circular reference as there is no need for it - and I suppose this is one good way to find it :slight_smile:

1 Like

I thought context.myname was deprecated.
The circular reference isn't necessarily an issue. Perhaps timers have circular references.
However there is a better pattern for achieving what @hazymat is trying to achieve, I just can't find an example of it immediately.

This was also my understanding :slightly_smiling_face: but it seems to be valid.

1 Like

It is deprecated because anything stored that way will not get persisted.

However, it still has its uses - such as this precise scenario where it is meaningless to persist a timer id object.

1 Like

Hi Colin, you have been enormously helpful so far, do let me know if it comes to mind. Would appreciate your input.

I don't profess to be a superb architect of code (! lol - I'm pretty crap) and was surprised nobody else made a really simple delay based number fader like this hence resorting to giving it a go myself. So any input is greatly appreciated.

Indeed, I could not find a node or flow in Node-RED library that address this need.

The subject of dimming light has already been discussed once in our forum and people provided a lot of insights and suggestions.

This smoothing problem resembles somewhat the capabilities that CSS give us to "ease" transitions.

It is feasible to build a solution without using timer functions (setTimeout , setInterval) since the smoothing factor depends only on the time elapsed since the beginning of the fading (only subtracting current time from the start time). Plenty of smoothing functions can be created based on easing functions or bezier curves.

1 Like

I knew I had done something very similar, and @Andrei reminded me. In the flow he linked to I posted a function node that does something very similar that almost does what you want.

[{"id":"5f8de1b.3f7c8a","type":"function","z":"6dc690a3.1abc88","name":"Slew rate limit","func":"// Limits the slew rate incoming payload values\nvar maxRate = 60  ;         // max slew rate units/minute\nvar period = 1000;          // period in millisecs to send new values, 1000 is 1 per second\n\nvar newValue = Number(msg.payload);\nvar timer = context.get('timer') || 0;\n// check the value is  a number\nif (!isNaN(newValue) && isFinite(newValue)) {\n    var target = msg.payload;\n    context.set('target', target);\n    // set last value to new one if first time through\n    var lastValue = context.get('lastValue');\n    if (typeof lastValue == \"undefined\" || lastValue === null) {\n        lastValue = newValue;\n        context.set('lastValue', newValue)\n    }\n    // calc new value\n    msg.payload = calcOutput();\n    // stop and restart the timer\n    if (timer) {\n        clearTimeout(timer);\n        context.set('timer', null);\n    }\n    timer = setInterval(function(){\n        // the timer has run down calculate next value and send it\n        var newValue = calcOutput();\n        if (newValue != context.get('lastValueSent')) {\n            context.set('lastValueSent', newValue)\n            node.send({payload: newValue});\n        }\n    },period);\n    context.set('timer', timer);\n    context.set('lastValueSent', msg.payload);\n} else {\n    // payload is not a number so ignore it\n    msg = null;\n}\nreturn msg;\n\n// determines the required output value\nfunction calcOutput() {\n    var lastValue = context.get('lastValue');\n    var target = context.get('target');\n    // set to current value if first time through\n    if (typeof lastValue == \"undefined\" || lastValue === null) lastValue = newValue;\n    var now = new Date();\n    var lastTime = context.get('lastTime') || now;\n    // limit value to last value +- rate * time\n    var maxDelta = (now.getTime() - lastTime.getTime()) * maxRate / (60 * 1000);\n    if (target > lastValue) {\n        newValue = Math.min( lastValue + maxDelta, target);\n    } else {\n        newValue = Math.max( lastValue - maxDelta, target);\n    }\n    context.set('lastValue', newValue);\n    context.set('lastTime', now);   \n    return newValue;\n}","outputs":1,"noerr":0,"x":433,"y":232,"wires":[["fb203846.5494b"]]},{"id":"fb203846.5494b","type":"debug","z":"6dc690a3.1abc88","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","x":650.5,"y":231,"wires":[]},{"id":"df5dd6ab.d3bdd","type":"inject","z":"6dc690a3.1abc88","name":"","topic":"","payload":"0","payloadType":"num","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":159.5,"y":176,"wires":[["5f8de1b.3f7c8a"]]},{"id":"2a7e98bc.cd7f18","type":"inject","z":"6dc690a3.1abc88","name":"","topic":"","payload":"10","payloadType":"num","repeat":"","crontab":"","once":false,"onceDelay":0.1,"x":158,"y":237,"wires":[["5f8de1b.3f7c8a"]]},{"id":"381ece91.b0c4a2","type":"ui_slider","z":"6dc690a3.1abc88","name":"","label":"slider","group":"41737f1.c5e588","order":0,"width":0,"height":0,"passthru":false,"topic":"","min":0,"max":"10","step":1,"x":172.5,"y":304,"wires":[["5f8de1b.3f7c8a"]]},{"id":"41737f1.c5e588","type":"ui_group","z":"","name":"Default","tab":"bb9b58bd.29621","disp":true,"width":"6"},{"id":"bb9b58bd.29621","type":"ui_tab","z":"","name":"Home","icon":"dashboard"}]

Again, though, I suggest not saving the timer id in persistent store. Make sure that if you are using the local file store for context that you also provide a memory store and use that.