Adding errors event listener

[ofirpaBrowsi]

Type of change

• Feature

Description of change

Added event emitter on every error logged to the console.

Using this listener will provide a way to get debug events without the need of 'debug mode'

Usage:
pbjs.onEvent('auctionDebug', (debugData) => { //Do stuff with the error data });

The idea came from the need to track our clients prebid errors, using this single event we can log prebid errors to our servers, notice their existence and fix them ASAP.

[patmmccann]

Do we need a test similar to test/spec/AnalyticsAdapter_spec.js line 75?

[GLStephen]

@ofirpaBrowsi this seems like a good change. I'd like to see one small change in addition to the feedback from @patmmccann. It is likely that someone may want to log other events later. It would be nice to change this to some variant of the below. This is pseudocode so doublecheck the syntax downstream.

events.emit(CONSTANTS.EVENTS.AUCTION_DEBUG, {type: 'ERROR', arguments: arguments});

This is a small change to allow some flexibility later with the goal of eventually allowing any other console debug to be sent through this channel, without adding another event type.

[patmmccann]

I added my suggestion on ofirpaBrowsi#1

[patmmccann]

fascinating behavior here; sigmoid and prebidanalyticsmanager are creating errors in their test spec which now go against the event count in those specs. fixed on ofirpaBrowsi#2

[ofirpaBrowsi]

@patmmccann Haven't noticed your fix PR (ofirpaBrowsi#2). I've Fixed it as same as you did though.

[ofirpaBrowsi]

@harpere @GLStephen any concerns?

[ofirpaBrowsi]

@harpere What should I do with the change request?

[ofirpaBrowsi]

Hi @harpere, Please help merging this PR?

[gpolaert]

What about naming it "onError" instead?

[ofirpaBrowsi]

What about naming it "onError" instead?

@GLStephen @harpere What do you think?

[harpere]

I like the onError suggestion. @GLStephen ?

[harpere]

what's there LGTM, though note we should consider the onError recommendation mentioned earlier.

[GLStephen]

@harpere @ofirpaBrowsi @gpolaert Semantically onError, or ON_ERROR, makes sense but ultimately we could send any of the debug info, so ON_DEBUG is maybe even more clear, at which point the current AUCTION_DEBUG is pretty close. The names of the whole array of events has never followed an "on" semantic though I agree it would have been arguably better.

I don't have heartburn about either approach at this point. @harpere your call

[harpere]

@ofirpaBrowsi - Let's change it so adding "INFO" or "WARN" later makes more sense. Though "onError" doesn't follow the existing format (EXISTING_FORMAT). Maybe just "ERROR"? or "AUCTION_ERROR"?

[patmmccann]

AUCTION_ERROR was the original commit; we've come full circle :)

[harpere]

AUCTION_ERROR was the original commit; we've come full circle :)

funny!

[GLStephen]

@patmmccann @harpere We have indeed. Too funny. To be fair my initial feedback was focused around making the change to process any of the debug/error related debug levels without another event being created. The name change of the actual event was a more subjective thing.

[patmmccann]

@ofirpaBrowsi if you change the event name lmk so I can change the docs pr prebid/prebid.github.io#2255

[harpere]

@ofirpaBrowsi will you make the change?

[ofirpaBrowsi]

@ofirpaBrowsi will you make the change?

@harpere I think we can use the current name (AUCTION_DEBUG) as it is not only for errors

[harpere]

@harpere I think we can use the current name (AUCTION_DEBUG) as it is not only for errors

@ofirpaBrowsi The event only fires from inside logError()

[harpere]

we just spoke about this in the Prebid JS committee meeting and there was agreement that we should change the name of the event.

[ofirpaBrowsi]

@harpere I think we can use the current name (AUCTION_DEBUG) as it is not only for errors

@ofirpaBrowsi The event only fires from inside logError()

Correct, but we planned to use this event on other debug events too (we added the 'TYPE' prop for this)
Anyway, I will change it as you wish, but please lmk what is the best name from your opinion.

[harpere]

@ofirpaBrowsi - Sorry for the disconnect. I think that's fine too (and don't think anyone else in the meeting would object), but if we do it that way, let's add the other event Types - WARN, INFO, MESSAGE (add to those log functions).

[ofirpaBrowsi]

@ofirpaBrowsi - Sorry for the disconnect. I think that's fine too (and don't think anyone else in the meeting would object), but if we do it that way, let's add the other event Types - WARN, INFO, MESSAGE (add to those log functions).

@harpere No problem, will be done, please note that the event emitter is using the 'logMessage' function so we could not implement AUCTION_DEBUG to this log type as it will cause infinite loop.

[harpere]

@harpere No problem, will be done, please note that the event emitter is using the 'logMessage' function so we could not implement AUCTION_DEBUG to this log type as it will cause infinite loop.

thanks @ofirpaBrowsi makes sense.

[GLStephen]

@harpere @ofirpaBrowsi can we/I merge this?

[harpere]

was hoping to get the event added to the other log functions, but that can be done later. yes, let's merge.