Fix buffered chan event system

[ankur22]

What?

This removes the default case for when reading done events from the extension or when sending events to the extension.

Why?

If we drop events due to the channel being full and there being a default case, then we can end up in a situation where either the event system indefinitely waits since incoming done events were not handled by the event system or the extension waits indefinitely if the event was dropped when sending it the event.

Checklist

• I have performed a self-review of my code.
• I have added tests for my changes.
• I have run linter locally (make lint) and all checks pass.
• I have run tests locally (make tests) and all tests pass.
• I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

grafana/xk6-browser#1440

[inancgumus]

LGTM 👍 One point to keep in mind: More calls to Done than the number of subscribers will again block :)

[ankur22]

One point to keep in mind: More calls to Done than the number of subscribers will again block :)

Yeah, that's a good point. I hadn't considered the reason why there was a default in the first place.

Having looked at this again, I still feel that this is the correct approach. Dropping done events or the events going to the extension will mean that one of the two will misbehave, either the extension will hang indefinitely or k6 will.

In the extension side (i.e. in browser) we could call e.Done() on a goroutine for the exit event. We don't want the browser module to hang around waiting for e.Done() to complete, so feels like a safe option. In my tests so far i've not needed to do this though, so i'd avoid doing that for now.

There is the chance that the subscriber could have called subscribe twice and handle the same subscription in the same goroutine, this could cause a deadlock (I think). This feels like user error though.