agsh / onvif

ONVIF node.js implementation
http://agsh.github.io/onvif/
MIT License
692 stars 234 forks source link

Two ways to intercept discovery errors #204

Closed bartbutenaers closed 2 years ago

bartbutenaers commented 2 years ago

Dear,

I see that in the latest version, there is a new way (1) to intercept discovery errors. It is not clear to me how this differs from the existing way to handle discovery errors (2):

image

Could somebody please explain a bit more in detail what the difference between both types of errors is?

Thanks a lot for maintining this library !!! Bart

RogerHardiman commented 2 years ago

I added Number 1 (on Error) because I had a device on my network that was not a camera, but returned some XML that caused an exception to be thown in the library. It was on a remote site so don't have access to now, but I think it was a NAS storage device. The workaround for me was the Discovery.on('error')

I'm not sure what you'd get in Number 2 without re-reading the code.

bartbutenaers commented 2 years ago

Hi @RogerHardiman, Thanks for the clarification!

I'm not sure what you'd get in Number 2 without re-reading the code.

I had a quick look at the probe Discovery function:

  1. I see on line 93 that the callback function is called with an error variable, for socket related errors:

    socket.on('error', function(err) {
      Discovery.emit('error', err);
      callback(err);
    });

    And in that case your new 'error' handler will also be called.

  2. On line 108 only your new error handler will be called for response related problems:

    if (err || !data[0].probeMatches) {
      errors.push(err || new Error('Wrong SOAP message from ' + rinfo.address + ':' + rinfo.port, xml));
      /**
      * Indicates error response from device.
      */
      Discovery.emit('error', 'Wrong SOAP message from ' + rinfo.address + ':' + rinfo.port, xml);
    } 
    else {
      // Here the response is parsed and added to the 'cams' array
    }

    The callback function is not being called here with an error parameter. Instead those response related errors are added to the errors array (which is used in step 3).

  3. On line 171 the device data map will only be passed to the callback function, if the errors array is empty.

    callback(errors.length ? errors : null, Object.keys(cams).map(function(addr) { return cams[addr]; }));

    It seems to me that in case of a non-empty error array, that the callback function is called with no errors and no device data. Or am I mistaken

So it seems to me that:

I don't understand point 3. Suppose you have 2 camera's and 1 NAS. That NAS returns a bad response, which is pushed to the errors array. But that would mean that you would receive no data about your 2 camera's, due to point 3. But that would mean that a single bad response from your NAS would result in none of the camera's being discovered (due to point 3). Does it behave like that in your case?

But it might be that I complete misunderstand the error handling mechanism, when having a look at the example on the readme page:

onvif.Discovery.probe(function(err, cams) {
// function will be called only after timeout (5 sec by default)
    if (err) { 
    // There is a device on the network returning bad discovery data
    // Probe results will be incomplete
    throw err;
  }
    cams.forEach(function(cam) {
        cam.username = <USERNAME>;
        cam.password = <PASSWORD>;
        cam.connect(console.log);
    });
});

Because here the err variable is explained as bad response, while I thought it are only socket errors. Moreover if the èrrors array contains a single item, then both parameters (err and cams) would be null or undefined. So then cams.forEach would crash. I don't understand how this can work with your NAS example ...

Sorry for digging into this. I'm asking those questions because I have a user that has reported that none of his camera's is being detected. So looking for something that could explain this ...