fix(config-cat): Rework error reporting #1242
Conversation
Signed-off-by: Adam Simon <[email protected]>
| this._hasError = false; | ||
| this.events.emit(ProviderEvents.Ready); | ||
| this._isProviderReady = true; | ||
| setTimeout(() => this.events.emit(ProviderEvents.Ready), 0); |
There was a problem hiding this comment.
Is there a reason to not emit this event right away? Also, this will likely trigger a re-render in the React SDK but it should no longer trigger a re-render loop.
There was a problem hiding this comment.
It's just that it doesn't feel right to invoke unknown user code during the evaluation of a feature flag, when normally that code would be invoked asynchronously, on the client's "background" polling loop. (As I mentioned in the related issue, the necessary event is not emitted by ConfigCat client at the moment but it's planned to be implement soon.)
Regardless, I'm ok with emitting the event synchronously if you say it's fine.
There was a problem hiding this comment.
The event would take two event loop cycles to run but I don't think it's a big deal either way. It's up to you.
There was a problem hiding this comment.
Well, then I'd leave it as is.
Signed-off-by: Adam Simon <[email protected]>
|
Sorry for the delay @adams85, many of us were at KubeCon last week. |
This PR
Attempts to fix #1212 by reworking error reporting. (More on the reason for the change in the related issue.)
Related Issues
Fixes #1212
Notes
This PR is not meant to be a complete solution, more of a quick fix to the related issue. It should be revised and improved later.
Follow-up Tasks
n/a
How to test
n/a