Commands which replies -USAGE aren't handled

[OscarF]

When executing a command (but just some commands) with the wrong syntax (ex: originate FOOBAR) the reply is neither "+OK" or "-ERR" but instead "-USAGE:". This isn't at all handled (https://github.com/englercj/node-esl/blob/master/lib/esl/Parser.js#L198-L206).

The spec doesn't say anything about that and the API replies aren't really standardised. I think that we can assume that "-USAGE:" means false for practical reasons.

[OscarF]

Apparantly I didn't do my research well enough. Seems like BACKGROUND_JOB events don't have a "Reply-Text" header but instead uses the event body.

The originate command either has a body of "+OK UUID" or "-USAGE: usage". How do you suggest it should be handled?

[englercj]

So, the parsing you patched in this PR was really there for auth messages so I could do this:

https://github.com/englercj/node-esl/blob/master/lib/esl/Connection.js#L522

I decided to put it in the parser, so the user also had an easy way to grab a state header from reply text on any event. I like merging this in so all events get that similar benefit for USAGE

That being said, the library does not handle error responses for you, for any events (except auth). You are expected to check the OK/ERR state yourself, it just helped with Reply Text by parsing the state and message out for you.

We can do that for body as well, but that gets a little more hairy. This parsing in Reply-Text works because I make the assumption that only a reply state/message will be in there; not sure I can make the same assumption about bodies...

If we were to do it, I would do something like this:

    //try and massage an OK/Error message
    var reply = event.getHeader('Reply-Text') || event.getBody(); //fall back to body on empty reply text
    if(reply) {
        if(reply.indexOf('-ERR') === 0) {
            event.addHeader('Modesl-Reply-ERR', reply.substring(5));
        } else if(reply.indexOf('-USAGE:') === 0) {
            event.addHeader('Modesl-Reply-ERR', reply.substring(8));
        } else if(reply.indexOf('+OK') === 0) {
            event.addHeader('Modesl-Reply-OK', reply.substring(4));
        }
    }

This should be fine for most cases.

[OscarF]

I realized my patch didn't solve my issue, unfortunately after a submitted it. I think error handling in general is a little bit difficult as the callbacks only receive an event and not the standard node arguments of (err, data).

Do you think it is a good idea to add error handling to the library in general or do you want to keep it as it is and let the handling be up to the user?

[englercj]

@OscarF It is a good argument that the library passes only an event and not the normal callback(err, data) that node usually has. The reason I chose not to do that is because I wanted this to be a very low-level library, one that is (for the most part) not context aware. I wanted this to be how you spoke to the ESL socket on FSW, but not how you interpreted that data.

Think of it like HTTP, if you get an HTTP response that is 404 Not Found, and had a body of { "error": true, "message": "Can't find X" } the HTTP library will not report an error, because it is the interpretation of that message that says something is wrong not the transmission. That is kind of what I was trying to accomplish here. There is no context awareness, only events that are moving through the pipe and actual errors from FSW. There are also not really "errors" I can pass back into a specific callback, when I do get an error from fsw I don't really know what message/call caused it; which is why there is only the generic error event.

I'm fine with the code block I posted above that will add an OK/ERR header to hint at the event state, but it is really up to the application to interpret that data how they want to. Hopefully with the block above you should be able to do:

conn.on('some_response_event', function(evt) {
    if(evt.getHeader('Modesl-Reply-OK')) {
        // everything is OK for this reply
    }
});

Does that make sense?

[englercj]

Closing this since I haven't heard anything in a while. Let me know if I need to reopen.