Need help with this error

I am seeing this error:

{"_msgid":"772966b40bfa3282","_event":"node:eafdfa87.79d67","error":{"message":"TypeError: Cannot read property 'colour' of undefined","source":{"id":"fed40e07.141c68","type":"function","name":"Ack event *","count":1}},"_error":{},"settings":{"input":"2022-09-20T00:37:11.134Z","input_format":"","input_tz":"Australia/Sydney","output_format":"YYYY-MM-DD HH:mm:ss","output_locale":"en_AU","output_tz":"Australia/Sydney"},"time":"2022-09-20 10:37:11","payload":{"message":"TypeError: Cannot read property 'colour' of undefined","source":{"id":"fed40e07.141c68","type":"function","name":"Ack event *","count":1}},"topic":"ERROR_REPORT/TelePi/Local Readings"}

Important part:

TypeError: Cannot read property 'colour' of undefined

So looking at the sauce (sorry)

Function node with code:

//  2022 07 16  To fix undefined errors.
if (msg.payload.colour == undefined)
    msg.payload.colour = "red";


var x = msg.payload.colour;
var who = msg.payload.device;
var led_msg = msg.payload.led;
var event = msg.payload.event;      //  2022 02 04  Added.
var more = flow.get("more_messages") || 0;
var msg2 = {};

//  2022 01 30  Still giving errors.
//  2022 02 04  Looking good.
var led = led_msg ? parseInt(led_msg.substring(4,5)) : 0

//  See top of code
////  2022 02 25  Fixing problem is msg.payload.colour is not set.  (Probably at boot)
//if (x == undefined)
//    x = "red";

node.warn("X " + x);
node.warn("Who " + who);
node.warn("led " + led_msg);


if (more > 0)
{
//    x = "orange";
    //
    msg2.payload = {"background":"red","color":"black","label":"MORE"};
//    msg2 = {"background":"red","color":"black","label":"MORE"};
}

if (more == 0)
{
    //
    node.warn("MORE = 0");
//    msg2.payload = {"background":"darkgreen","color":"white","who":"ACK"};
    msg2.payload = {"background":"darkgreen","color":"white","label":"ACK"};
    led = context.get("old_led") || 0;
}


if (who == "All clear")
{
    //
    node.warn("ALL CLEAR");
    msg = {
        "payload": "rgb," + led + ",0,0,0",   //  Needed to handle varying led numbers
        "who":"All clear",          //  Depricated 2021 12 04
        "wipe":"CLEAR"
    };
    
    msg1 = {payload: '<font color = '+x+'><i class="fa fa-bullseye fa-2x"></i></font>',"who": "All Clr "}; //  Should be `event`?
    //msg1 = {payload: '<font color = '+x+'><i class="fa fa-bullseye fa-2x"></i></font>',"who": event};
    //
}
else
{
    //
    msg.payload = led_msg;
    msg1 = {payload: '<font color = '+x+'><i class="fa fa-bullseye fa-2x"></i></font>',"who": who};
    //msg1 = {payload: '<font color = '+x+'><i class="fa fa-bullseye fa-2x"></i></font>',"who": event};

//    node.warn("Colour " + x);
//    node.warn("Who " + who);
//    node.warn("Led " + led);
//    node.warn("More " + more);

    context.set("old_led",led);

}

return [msg,msg1,msg2];

Forgive all the comments.

But looking at the top bit of it:

//  2022 07 16  To fix undefined errors.
if (msg.payload.colour == undefined)
    msg.payload.colour = "red";


var x = msg.payload.colour;

Unless colour is from somewhere else.

But those lines should stop the problem for x getting msg.payload.colour if it not defined.

Thoughts?

It is exactly as it says. Cannot read a property named colour from undefined. I.e. msg.payload is empty/null/undefined

Your test is inadequate.

If msg.payload is undefined how can it check a sub property named colour? It can't.


You probably want something like this at the very top of the function...

if (msg.payload == null || typeof msg.payload !== "object") {
  node.error("payload must be an object", msg)
  return null
}
2 Likes

But the idea is that the payloads are structured correctly.

But (from the machine's reboot) I see this:
{"_msgid":"772966b40bfa3282","_event":"node:eafdfa87.79d67"}
And that is way not what is supposed to coming in.

So the sender is the node in the message?

But I will have to put something like what you showed me to catch any other messages sneaking through, into the node.

Make sure you understand why there is no payload, so you can fix that, or cope with it properly.

Yes, that is a good idea, but I don't think it is in my skill set.

Tracing the message, it only points to a link out node.

Do you mean you have a message coming into a function node and you don't know where it is coming from? That is a disaster. How can you expect to have a stable system if you don't understand where messages are coming from?

Add a function node in the output of each node that feeds it (or feeds the link node) with a warn() function if the payload is missing and you will identify it.

Well, when I looked at the message - as given to me by the system - and searched for the originating node: all I was given was a link out node.

So that doesn't really help me, other than that part of the flow is sending it.

I didn't make node-red so I can't answer the question as I see it.

Screenshot from 2022-09-20 18-58-58

Is the link In node connected to the input of your failing function? If so then presumably it is coming from one of the link out nodes feeding that link in. So do what I suggested in my last post.

The message has a this message is coming from node-id.

You see I search for it and there is the link in and link out nodes.

Obviously it won't be the link in node, but saying it is form a link out node?

The upstream nodes as shown.

Is that node connected (via link in) to the function node?

That and the.....

The function node that spits the error is connected via a link out and link in node and a subflow to another function node shown above called Identifyh SOM/EOM EVENT.

I am talking about the inputs to the function node, it is there that the payload is missing

Yes, the node which reports/creates the error and part of who's code I posted at the start:

The input is from (via other link nodes) the node identify SOM/EOM EVENT

code:

if (msg.topic == "TIMESTAMP")
{
    return msg;
}
msg2 = {};

if (msg.type == "SOM")
{
    //  SOM
    msg2.payload = 
    {
        "device":msg.payload,
        "what": "SOM message",
        "event": "SOM message",
        "led":"rgb,6,118,15,4",
        "colour":"orange"
    };
}
if (msg.type == "EOM")
{
    //  EOM
    msg2.payload = 
    {
        "device":msg.payload,
        "what": "EOM message",
        "event": "EOM message",
        "led":"rgb,6,118,0,30",
        "colour":"orange"
    };
}

return msg2;

And now looking at that code:

if (msg.topic == "TIMESTAMP")
{
    return msg;
}

May be a problem.
But in this (newer) specific case: payload would be set.
But I guess I will need to address that problem also.

if (msg.topic == "TIMESTAMP") {
    //correctly handled case where msg topic is TIMESTAMP, works as expected. 
    return msg;
}

// we are here because of "msg.topic" is not "TIMESTAMP"


// declared an empty object.
let msg2 = {};


// handling case where msg.type is string "SOM"
if (msg.type == "SOM") {
    //  SOM

    // previously declared object "msg2" gets property "payload".
    msg2.payload =
    {
        "device": msg.payload,
        "what": "SOM message",
        "event": "SOM message",
        "led": "rgb,6,118,15,4",
        "colour": "orange"
    };
}

// if msg.type was something else or missing, the "msg2" is still an empty object


// handling case where msg.type is string "EOM"

if (msg.type == "EOM") {
    //  EOM

    // previously declared object "msg2" gets property "payload".
    msg2.payload =
    {
        "device": msg.payload,
        "what": "EOM message",
        "event": "EOM message",
        "led": "rgb,6,118,0,30",
        "colour": "orange"
    };
}

// unhandled cases
// msg.type is missing  --->  the "msg2" is an empty object
// msg.type is not "EOM" or "SOM" ---> the "msg2" is an empty object

// if empty object should not be passed downstream ... what can be done here?

return msg2;
1 Like

Thanks for that.

What I wrote was an early attempt at that, but old habits are seen with var rather than let.

(I'll now reply to all)

Ok, I am going to try and look at what is going on now while my hay fever is behaving.

The first bit that tests for TIMESTAMP is (probably) not the best but an artifact of other nodes I wrote.

The upstream node (Identify SOM/EOM EVENT) may be a duplicate node from part of the flow where it is logged.
That part of the flow also gets Timestamps (I press a button and it time stamps (marks) the log with the time.)

This was problematic as when this node (or nodes similar) got the timestamp message, they would spit their dummy and error. So I put that part at the top to simply pass on the message.

Things evolve and so that bit of code was propagated to others to avoid that problem.
But! Looking at where THIS node is, the timestamp doesn't need to be passed on.
So I guess I could simply return rather than return msg. But that is beside the point for the message I caught originally.

Some of this code is from when I was starting in NR and I hope I have grown since then. But (sorry) old things hang around and re-enforce the wrong habits when I am forced to look back and fix problems in the node/code.

I'll stop typing and action what I said above:
I'll see if I can find what is causing the error.

Thanks to all.

(I'm stuck)

Ok, things aren't quite as simple as I would like, but what else is new? :frowning:

So looking more at the node which is still getting the wrong kind of msg.

This is the start of the code:

if (msg.payload == null || typeof msg.payload !== "object")
{
    node.error("payload must be an object", msg);
    return null
}

Alas, the node.error is somewhat lost in the messages.
(I've just realised I log the error messages)

But while I'm here.

Would this work:

if (msg.payload == null || typeof msg.payload !== "object")
{
    flow.set("ACK_ERROR",msg.payload);
    node.error("payload must be an object", msg);
    return null
}

So when that bit of code is run it sets the flow variable "ACK_ERROR" to what the payload is?

Just wondering.

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.