firebase icon indicating copy to clipboard operation
firebase copied to clipboard

Better API for Watch / SSE Streaming

Open ereyes01 opened this issue 10 years ago • 3 comments

Currently Watch accepts a stop channel and returns an event channel. I'd like to make a simpler API that emulates bufio.Scanner. This should wrap the existing implementation to preserve binary compatibility, and to not break users who might need to select on multiple watchers.

Below is an example of what the new API would look like:

Initialize a watcher, and clean it up when we're done with it:

watcher, err := client.NewWatcher(unmarshaller)
if err != nil {
    // handle error
}
defer watcher.Close()

Iterate through events, and handle errors:

for watcher.Listen() {
    if err := watcher.ParseError(); err != nil {
        log.Println("Malformed event: ", err)
    }

    event := watcher.Event()
    // process event
}

if err := watcher.Error(); err != nil {
    // handle error
}

ereyes01 avatar Nov 04 '15 05:11 ereyes01

I might have talked myself out of implementing this feature after trying to get started on it. In the end, I think it will not make streaming/watching Firebase resources significantly easier. The examples above are misleading, as ParseError() and Error() have to be checked on each iteration of Listen(), just like you do with the channel. Receiving either kind of error does not imply the channel has closed.

Stuffing everything into a StreamEvent struct spit out from a channel may seem unwieldy, but it's the simplest way I know to accurately model all the possibilities Firebase can throw at you when streaming changes to a resource.

ereyes01 avatar Jan 30 '16 20:01 ereyes01

https://groups.google.com/forum/#!topic/firebase-talk/TRO_bEcWhII

@inlined proposed a nice way to design this by making the Watcher object contain a json.Decoder object. This would get rid of the whole EventUnmarshaller business and allows json parse errors to be handled outside of the library, instead of exposing it via the UnmarshallerError.

I'm thinking through the possible error + disconnection scenarios to make sure that the design accounts for all the possibilities, which are (as far as I know):

  • The Firebase connection dies
  • You get a cancel event (https://www.firebase.com/docs/rest/api/#section-streaming-cancel) because you no longer have permission to read the location you're watching
  • You get a auth_revoked event (https://www.firebase.com/docs/rest/api/#section-streaming-auth-revoked) because the token you were using has expired, or the secret got revoked, or something like that.

In particular, the cancel error is not necessarily a fatal error. Permissions can change to de-authorize you from reading a path, but could also change later to re-authorize you, thus conceivably allowing you to reuse your connection (firebase doesn't seem to have a uncanceled event though). I can't think of a way you could recover from auth_revoked at the moment.

Let's operate on the assumption that a cancel event is a fatal error, which is probably true in most practical cases.

I think another important part we have to account for is to allow the caller to see the event type and the path. These two pieces of information might be necessary to properly decode the event's data payload.

With those things in mind, here's my swing at how this API could look like:

watcher, err := client.Child("some/place").NewWatcher()
if err != nil {
    // handle error
}
defer watcher.Stop()

var (
    streamErr error
    foo Foo
    event firebase.WatchEvent
)

for {
    event, streamErr = watcher.NextEvent()
    if streamErr != nil {
        break
    }

    // event.Type is either "put" or "patch"
    // event.Path has the path that this event is for  

    if err := event.DecodeData(&foo); err != nil {
        log.Println("Couldn't parse event:", err)
        continue
    }

    // do something with foo
}

log.Println("Finished with err ==", streamErr)

When I get some time, I'll revisit this and hammer this out. In the meantime, if anyone has feedback, please feel free to provide it.

Thanks, Eddy

ereyes01 avatar Feb 05 '16 05:02 ereyes01

As illustrated in the discussion in #9, event.Type and event.Path are important to determining what you can successfully unmarshal into an object via the DecodeData method.

ereyes01 avatar Nov 28 '16 21:11 ereyes01