FlowFuse / flowfuse

Connect, collect, transform, visualise, and interact with your Industrial Data in a single platform. Use FlowFuse to manage, scale and secure your Node-RED solutions.
https://flowfuse.com
Other
279 stars 63 forks source link

Exception caused by Device.Team is null #3221

Closed sentry-io[bot] closed 9 months ago

sentry-io[bot] commented 10 months ago

Sentry Issue: FLOWFUSE-BACKEND-2A

TypeError: Cannot read properties of null (reading 'hashid')
  File "/usr/src/forge/app/node_modules/@flowforge/flowforge/forge/db/controllers/Device.js", line 77, in Object.sendDeviceUpdateCommand
    app.comms.devices.sendCommand(device.Team.hashid, device.hashid, 'update', payload)
  File "/usr/src/forge/app/node_modules/@flowforge/flowforge/forge/comms/devices.js", line 108, in DeviceCommsHandler.handleStatus
    await this.app.db.controllers.Device.sendDeviceUpdateCommand(device)
  File "node:internal/process/task_queues", line 95, in process.processTicksAndRejections
Pezmc commented 10 months ago

Checking the device model, on a cursory glance, it looks like a device should always have a team. Can anyone explain why we'd be receiving status events from devices without teams? Or are they perhaps just not being loaded by the db request?

Steve-Mcl commented 10 months ago

@Pezmc does the sentry distinguish dev time issues (i.e. me on my dev machine, breaking things) from real world (out there / customers) issues?

Pezmc commented 10 months ago

@Steve-Mcl It logs the environment and we typically filter out issues raised in development, this one in question has happened 3200 in production since December the 12th.

hardillb commented 10 months ago

There are a number of devices still in the database that were not deleted when the team was deleted.

Pezmc commented 10 months ago

Seems like we are still getting status check-ins from them then!

Steve-Mcl commented 9 months ago

I have checked through the calls to app.comms.devices.sendCommand via sendDeviceUpdateCommand in forge/app/node_modules/@flowforge/flowforge/forge/db/controllers/Device.js" @ line 77

As far as I can see device is always a fully loaded model (e.g. loaded via await app.db.models.Device.byId(targetDevice.id)) with the required associations

So if these ARE orphaned devices (no team associated) we should probably contain that?

Perhaps something like:

            if (device.Team) {
                app.comms.devices.sendCommand(device.Team.hashid, device.hashid, 'update', payload)
            } else {
                // reload the device with the team association
                const fullDevice = await app.db.models.Device.byId(device.id)
                if (!fullDevice || !fullDevice.Team) {
                    app.log.warn(`Failed to send update command to device ${device.hashid} as it has no team association`)
                    return
                }
                app.comms.devices.sendCommand(fullDevice.Team.hashid, fullDevice.hashid, 'update', payload)
            }

It would at least:


As for orphaned devices, they should be picked up by #2569

knolleary commented 9 months ago

Okay - I got our sentry issues mixed up. This PR addresses an error message we haven't seen in production for over a month. This is not the one I thought it was.