Feature: event system and impression data events #113
Conversation
| callback(evtConfig); | ||
| EventConfig = evtConfig; | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Is there something that can be raised here? This looks pretty harmless to me
There was a problem hiding this comment.
This particular one is probably very harmless, it's the callback where you configure all the handlers for the various events. But anything started in here that fails on a file lock or whatever will crash the startup of Unleash
| Variant = variant | ||
| }); | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Same as above, I can't see an obvious exception that could be caught here, is this just being defensive?
There was a problem hiding this comment.
Here I'm trapping event emitting, if your logger fails for some reason for instance, that won't crash harder than an error in the log
There was a problem hiding this comment.
Sure, I can live with that, seems polite to log an error if something super unexpected happens here
|
Thanks @sighphyre for review and questions. I've attempted a few more details in response. Perhaps this is a code area that could do with some comments 🤔 I'm trapping exceptions here since these two callbacks are code coming from outside Unleash, but I'm open for anything :) |
Description
Aims to add an event system to the SDK and a first implementation for impression data/events: https://docs.getunleash.io/advanced/impression-data#impression-event-data
Fixes # (issue)
#108
Type of change
How Has This Been Tested?
Checklist: