NFIBrokerage / spear

A sharp EventStoreDB v20+ client backed by Mint :yum:
https://hex.pm/packages/spear
Apache License 2.0
85 stars 14 forks source link

Streaming events returns an error when from is greater than the last event number #67

Closed bminevsb closed 2 years ago

bminevsb commented 2 years ago

I found this problem while experimenting with Spear.

Let say we have 30 events in stream with uuid stream_id (with numbers 0,...,29). This returns an error instead of []

MySpearClient.stream!(stream_id, from: 30)

After some debugging, I found that this is caused by Spear.Reading.Stream.wrap_buffer_in_decode_stream/3 not handling this case. The case statement receives this:

{{:"event_store.client.streams.ReadResp", {:last_stream_position, 9}}

which produces a FunctionClauseError once it propagates down to unfold_continuous.

To handle this I think it should do something like:

    case unfold_chunk(buffer) do
      {Streams.read_resp(content: {:stream_not_found, Streams.read_resp_stream_not_found()}),
               _rest} ->
         []

-      {message, _rest} ->
+      {event() = message, _rest} ->
         Stream.unfold(
           %__MODULE__{state | buffer: buffer, from: message},
           unfold_fn
         )

-      nil ->
+      _ ->
         []
     end
   end
the-mikedavis commented 2 years ago

Thanks for the great write-up!

I agree - this should return []. The change you suggest looks good but out of paranoia of what might match _, I would prefer to add a new clause similar to the top one:

{Streams.read_resp(content: {:last_stream_position, _}), _rest} ->
  []

What do you think, would you like to submit a PR?

bminevsb commented 2 years ago

Sure! From what I understand on success it can return, an event, :last_stream_position, or :first_stream_position: https://github.com/EventStore/EventStore/blob/master/src/EventStore.Core/Services/Transport/Grpc/Enumerators.ReadStreamForwards.cs#L126 :first_stream_position is returned if you call MySpearClient.stream!(stream_id, from: 31) in the example above. So I'll cover both cases.

bminevsb commented 2 years ago

That was fast! ⚡ Thanks for the quick response @the-mikedavis 🙇