Broken "Inputs" page in emoncms 10.1.1

franck102 · 28 June 2019 07:46

Hi all,

After upgrading my emonbase to emoncms 10.1.1 the Inputs page won’t load anymore:
Capture

Here are the details of my setup:

#### EMONCMS

Version: low-write 10.1.1
Modules: Administration | App v2.0.1 | Backup v2.1.0 | EmonHub Config v2.0.2 | Dashboard v2.0.3 | Device v2.0.0 | EventProcesses | Feed | Graph v2.0.1 | Input | Postprocess v2.1.0 | CoreProcess | Schedule | Network Setup v1.0.0 | sync | Time | User | Visualisation | WiFi v2.0.0

Git URL: https://github.com/emoncms/emoncms.git
Branch: * stable
Describe: v9.5.1-2081-g76ced7cb

Any suggestion?
Thanks!
Franck

franck102 · 28 June 2019 07:51

Javascript debugging seems to indicate that the page chokes on a missing device description…

borpin · 28 June 2019 08:00

Have you cleared the browser cache?

franck102 · 28 June 2019 08:39

Yes I had.

I went to mysql to fix it, somehow I had an input whose nodeid did not exist in the device table.
This is an input I created manually, in the previous version of the emoncms UI… I don’t have precise reproducible steps unfortunately.
I see JS code to try and ask the server to create missing devices, but somehow that didn’t kick in here…

Franck

borpin · 28 June 2019 10:42

I am seeing this error too.

@glyn.hudson @TrystanLea @emrys there seems to be a problem with the latest patch.

TrystanLea · 28 June 2019 12:03

Thanks @franck102 @borpin, Im cant replicate it here, however it sounds like it might be resulting from this line: emoncms/input_view.php at master · emoncms/emoncms · GitHub
could you try commenting that line out @borpin to see if the error goes away?

borpin · 28 June 2019 12:13

@TrystanLea - yes it goes away.
[edit]
is there a requirement to check and see if there is a device name specified?
[edit2]
Should the devices array be passed here.

github.com

emoncms/emoncms/blob/2497fd297b7fd3ccecb119be234a2bf7227c512a/Modules/input/Views/input_view.php#L299-L299

    
      
          draw_devices();

TrystanLea · 28 June 2019 12:38

The devices array is in the scope of the whole script, so does not need passing.

It sounds like the device does actually exist in the array but its missing the description property, hence its not creating a device at this point emoncms/Modules/input/Views/input_view.php at master · emoncms/emoncms · GitHub but somehow the description is not present before that…

Would you be able to put a console.log(devices) at this point and copy here what it prints to the javascript console? emoncms/Modules/input/Views/input_view.php at master · emoncms/emoncms · GitHub

Alternatively I can add a check for the description, but it may be not addressing the root cause.

borpin · 28 June 2019 13:25

It was just that the other function (next line) did do so - not consistent.

The purist in me says you should always check and never assume .

If it is empty would it be undefined?

borpin · 28 June 2019 13:28

Don’t know how to export the object. I think your assumption is false…

borpin · 28 June 2019 13:32

BTW, if I click on ‘new device’ there is no description field.

Robert.Wall · 28 June 2019 13:50

I thought that was called “defensive programming”.

franck102 · 28 June 2019 15:07

I can’t reproduce anymore, but when debugging I did find a funky device (different properties, missing description, and with the message that appears in Brian’s log), which seems to come from device_model.php.
The line Trystan points to assumes that the device is either null or correct, the above breaks that assumption…

Franck

franck102 · 28 June 2019 15:13

That’s definitely the line that triggers the exception, but just commenting it out will still leave you with malformed device objects in the array… so probably a defect waiting to happen

franck102 · 28 June 2019 15:39

The problem could well be here:

github.com

emoncms/device/blob/2ea6e744bbcdb0f15834b4a625934a6a095c0332/device_model.php#L240

    
      
          }
          
          
$devices = array();
          $deviceids = $this->redis->sMembers("user:device:$userid");
          foreach ($deviceids as $id) {
              $device = $this->redis->hGetAll("device:$id");
              // Verify, if the cached device contains the userid, to avoid compatibility issues
              // with former versions where the userid was not cached.
              if (!isset($device['userid'])) {
                  $this->load_device_to_redis($id);
                  $device = $this->get($id);
              }
              $device['time'] = $this->redis->hget("device:lastvalue:".$id, 'time');
              $devices[] = $device;
          }
          usort($devices, function($d1, $d2) {
              if($d1['nodeid'] == $d2['nodeid'])
                  return strcmp($d1['name'], $d2['name']);
              return strcmp($d1['nodeid'], $d2['nodeid']);
          });
          return $devices;

When getting the device list from Redis the code post-processes the array, calling this->get(), and it doesn’t check that that get() call was successful; instead it just inserts an error response in the device array…

Franck

TrystanLea · 28 June 2019 21:32

I’ve added a check for the description higher up in the input_view code, where the device list is loaded: https://github.com/emoncms/emoncms/commit/f25b1125b685858c26d65507cbaa4bceb61c8d03

What error response are you seeing? Sounds similar to what @freeyland is seeing here?: Uncaught reference error list format updated object

borpin · 28 June 2019 21:41

So this fix solves the issue but it seems none of the devices (on a standard EmonSD image) get populated.

Where should this information be entered?

I have to say I do not understand the ‘devices’ at all.

franck102 · 29 June 2019 06:33

I created a bogus input to reproduce the problem. The issue is here:

github.com

emoncms/emoncms/blob/bb12d7b6f01b4ed99b0645200821d0798d3b575a/Modules/input/Views/input_view.php#L276

    
      
          for (var z in data) inputs[data[z].id] = data[z];
          
          
// Assign inputs to devices
          for (var z in inputs) {
              // Device does not exist which means this is likely a new system or that the device was deleted
              // There needs to be a corresponding device for every node and so the system needs to recreate the device here
              if (devices[inputs[z].nodeid]==undefined) {
                  devices[inputs[z].nodeid] = {description:""};
                  // Device creation
                  $.ajax({ url: path+"device/create.json?nodeid="+inputs[z].nodeid, dataType: 'json', async: false, success: function(deviceid) {
                      if (!deviceid) {
                          alert("There was an error creating device: nodeid="+inputs[z].nodeid+" deviceid="+deviceid); 
                      } else {
                          $.ajax({ url: path+"device/get.json?id="+deviceid, dataType: 'json', async: false, success: function(result) {
                              devices[inputs[z].nodeid] = result;
                          }});
                      }
                  }});
              }
              if (nodes_display[inputs[z].nodeid]==undefined) nodes_display[inputs[z].nodeid] = true;
              // expand if only one feed available

The call to device->create returns this error object:

1. deviceid:
  * message: "Device already exists"
  *  success: false
  *  __proto__: Object

… and the code assumes it simply received a deviceId.

The problem is all over the place in device module: several of its functions return either an object (deviceid, device), or an error array (success: false, message: xxx).
And none of the usages of the module are testing for that.

I don’t have time to look into this further, maybe next week…

To reproduce, simply use this:

MariaDB [emoncms]> update input set nodeid='bogus' where id=292;

sudo systemctl restart redis-server

Franck

TrystanLea · 29 June 2019 12:41

Thanks @franck102, I’ve improved the implementation to check for the error message object in the result.

$.ajax({ url: path+"device/create.json?nodeid="+nodeid, dataType: 'json', async: false, success: function(result) {
    if (result.success!=undefined) {
        alert("There was an error creating device: nodeid="+nodeid+" message="+result.message); 
    } else {
        devices[nodeid].id = result;
        devices[nodeid].devicekey = "";
    }
}});

See: emoncms/input_view.js at master · emoncms/emoncms · GitHub
and commit https://github.com/emoncms/emoncms/commit/6d3b59a5b35757966c0a5689eddee6f4f33412ca.

This is merged in after another merge fixing an issue where the input view did not work if the device module was not installed - hence other changes that might not look familiar.