Replacing use of console.error with an event emitter

[jonastelzio]

I'm finishing up logging for my project, and there are a few places internally in the SRF framework where there are direct calls to console.error. Some of them looks mainly like debugging statements, but there are some that catches fatal errors, like this one:

https://github.com/drachtio/drachtio-srf/blob/master/lib/proto.js#L161

How would you feel about replacing (some) those with emitting an error event on some component? Could be on the request-object itself, or on the srf object.

Right now, to get these logged, I have to do hacky stuff like this:

const cError = console.error;
console.error = (s, e) => {
    log.error(s, e);
    cError(s, e);
}

I down for writing the code, but I'd like your input first.

[davehorton]

I could definitely use some advice/help on how to implement/integrate logging into the framework better. You are right, there are spots where I would like to generate a log message from within the framework and right now just using console, which I don't love. Here is another one:

https://github.com/drachtio/drachtio-srf/blob/1629d631136c7ecf60c8af28d2d8b9d9c39f15e4/lib/drachtio-agent.js#L220

I don't think we can always guarantee that there is a request object in scope, though, so I am not sure about that. Emitting an error on the srf object sounds a bit better, though at lower levels of the stack are not aware of the higher level abstraction that is the srf. Definitely open to suggestions.....

[jonastelzio]

@davehorton so, my immediate reaction would be that any error that's related to code that the implementor (me) has written, should be obtainable through an event on on the SRF instance.

So all errors in middleware, srf.invite and so on, should end up in srf.on('error') - or whatever, I believe that event is already occupied by errors in communication for inbound mode.

Errors that sits at a lower level of abstraction should probably be fine to emit to console.error instead. From my standpoint as an implementor, it should be fine getting caught by whatever logging I've got attached to stderr.

The extensive use of promises can make this a bit annoying to implement though, but with a few strategically placed await statements it'll probably be manageable.

[davehorton]

that sounds pretty reasonable. One thing I was wondering -- that initial link you gave, an error of that kind should have called your error middleware, if you had any, via this line:

https://github.com/drachtio/drachtio-srf/blob/1629d631136c7ecf60c8af28d2d8b9d9c39f15e4/lib/proto.js#L197

was that not happening?

[jonastelzio]

I'm not sure what error middleware is?

[davehorton]

it is middleware with 4 args instead of 3, one being an error argument. It is called only when an error is barfed somewhere down the stack:

srf.use((err, req, res, next) => ..

[jonastelzio]

Oh I didn't know that! That takes care of a great deal of the error handling then! I'll just try that out.