commanded / eventstore

Event store using PostgreSQL for persistence
MIT License
1.04k stars 142 forks source link

Dialyzer error when calling subscribe/2 with `name` option #267

Closed ryoung786 closed 8 months ago

ryoung786 commented 1 year ago

I've got a project that uses EventStore, and takes advantage of the dynamic naming for tests. When I run dialyzer on the project, it fails if there is a call to subscribe/2 that passes in a name: name option (ex: FooEventStore.subscribe("stream_id", name: :foo):

The function call will not succeed.

FooEventStore.subscribe(_stream :: any(), [{:name, _}, ...])

will never return since the 2nd arguments differ
from the success typing arguments:

(binary(), [{:mapper, (map() -> any())} | {:selector, (map() -> any())}])

Here's the relevant bit of source code from EventStore:

# lib/event_store.ex
@type transient_subscribe_option ::
        {:name, atom}
        | {:selector, (EventStore.RecordedEvent.t() -> any())}
        | {:mapper, (EventStore.RecordedEvent.t() -> any())}
@type transient_subscribe_options :: [transient_subscribe_option]

@callback subscribe(stream_uuid :: String.t(), opts :: transient_subscribe_options) ::
            :ok | {:error, term}

def subscribe(stream_uuid, opts \\ []) do
    name = name(opts)

    PubSub.subscribe(name, stream_uuid, opts)
end

# lib/event_store/pubsub.ex
@spec subscribe(
        EventStore.t(),
        binary,
        selector: (EventStore.RecordedEvent.t() -> any()),
        mapper: (EventStore.RecordedEvent.t() -> any())
      ) :: :ok | {:error, term}
def subscribe(event_store, topic, opts \\ [])

I think the issue is that transient_subscribe_options typespec allows name in the opts list , but the underlying call to PubSub.subscribe/3 does not. To fix this, we could either delete name from opts before passing it to PubSub.subscribe/3, or we could amend the @spec definition for PubSub.subscribe/3 to include the name option.

slashdotdash commented 8 months ago

Hopefully this has been fixed by #268.