agsh / onvif

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

Catching ONVIF SOAP Faults? #218

Closed ImPhantom closed 2 years ago

ImPhantom commented 2 years ago

First off, thank you for this extremely capable library, everything's working perfectly so far. But I just can't for the life of me find a way to catch the SOAP Faults that are thrown by parseSOAPString (being called by Cam._request) https://github.com/agsh/onvif/blob/a218a6b836b5088f7c953cca6a3835d0a99ab287/lib/utils.js#L101

Cause when I input an invalid username/password and it throws "Sender not Authorized", I'd love to catch that and return an invalid response instead of exiting the process.

How I'm creating the Cam instance:

function testConnection(host, port, username, password) {
    return new Promise(function(resolve, reject) {
        const camera = {};
        try {
            new onvif.Cam({
                hostname: host,
                port: port,
                username: username,
                password: password,
                timeout: 2500
            }, async function handle(err) {
                if (err) console.log(err);

                // Doing things here

                resolve(camera);
            });
        } catch (err) {
            console.log(err);
        }
    });
}

(my first ever issue anywhere, so apologies if I'm missing anything that would be helpful)

agsh commented 2 years ago

@ImPhantom Hi! This library is only callback-driven yet, so I think, you have some mistakes in your code

agsh commented 2 years ago

Also, Cam class inherits EventEmitter class. You can subscribe on error event. I hope it can helps you. Thanks for your response!

ImPhantom commented 2 years ago

Thank you for responding so quickly, especially around the holidays. But I'm aware its callback driven, but I checked out examples/example7.js and saw the use of "promisify" return the camera datetime/info requests as promise's so they could actually be returned synchronously from a function. So the code I have right now functions perfectly as long as you provide correct credentials/connection details. I'm just trying to prevent it from fully terminating the process.

I have tried subscribing to the error event on the created cam object, but it doesn't seem to be triggered on SOAP Faults, or at least not "Sender not Authorized".

RogerHardiman commented 2 years ago

The overall structure of the code to return a Promise is fine but there are no calls to reject() in the sample code you sent. Reject should be called when the onvif library callback has an 'err' value. You need one in the catch(). Without the reject() the code calling testConnection will never see the promise complete and provide the final value, unless your intention was to just use resolve() and pass back a null value or a good value. but there is no resolve() after the try/catch, so that fails too.

RogerHardiman commented 2 years ago
onvif = require('../lib/onvif');

function testConnection(host, port, username, password) {
        return new Promise(function(resolve, reject) {
                const camera = {};
                try {
                        new onvif.Cam({
                                hostname: host,
                                port: port,
                                username: username,
                                password: password,
                                timeout: 2500
                        }, function handle(err) {
                                if (err) {
                                    //console.log(err);
                                    reject(err);
                                    return;
                                }

                                // Doing things here

                                resolve("it worked");
                        });
                } catch (err) {
                        //console.log(err);
                        reject(err);
                }
        });
}

let connectionPromise = testConnection("192.168.1.15", 80, "baduser", "badpass");
connectionPromise.then((value) => console.log("OK    VALUE=" + value))
                  .catch((err) => console.log("ERROR VALUE=" + err));
RogerHardiman commented 2 years ago

The code above is an example of a working testConnection (tested with good and bad username/passwords with an Axis camera).

The main differences compared to your original are a) you don't need async before the callback function. You only need that when using await b) the if (err) needs a call to reject() and then Return so it does not move into 'Doing things Here'. c) you need a reject() in the Catch too d) The code that calls testConnection will get a Promise as the return value. A promise has two states. 1) the code inside the Promise is still executing 2) the code inside the Promise has finished doing what it is doing and called resolve(some_value) or reject(an_error). resolve is used to share a result. reject is used to share an error.

So when you have a Promise, you need to wait for the code inside the promise to finish doing whatever it was doing and for that you use .then() and .catch() The .then() contains an embedded function that is called when we have good data (ie the 'resolve()' was called inside the Promise. The .catch() contains an embedded function that is called when we have an error inside the Promise code (and the promise code called reject()

ImPhantom commented 2 years ago

@RogerHardiman Thank you for responding as well! I originally had rejects in most of those spots, but removed them to see where the error was actually being thrown before posting the issue. But before that I guess I wasn't never actually catching any of the errors in my Cam instance callback, once I started catching those it works great and lets me pass errors along. Rogers explanation made me look at my code and realize I was totally overthinking it all, Thank you all so much & Have a happy new year!

RogerHardiman commented 2 years ago

Hi @ImPhantom Glad I could help. Another tip is to use the await command in your javascript instead of the .then().catch() in the example I did above. It saves having to nest code inside the .then() handler.

This is how example7 works. Example 7 uses a library to wrap existing functions in the library and magically turn them into functions that return a Promise. Then Example7 can use 'await' to wait for the code inside the promise to finished.

Anyway, happy to help and have a good New Year too.