fix: Improve client quality and align with Go client#28
Conversation
…nvalid tuples
Stream.resource/3 only accepts {list, acc} or {:halt, acc} as return values.
Returning {:error, reason} was invalid and would crash the stream consumer
with an unclear error. Now raises the exception directly, which propagates
cleanly to the consumer and allows Stream.resource's cleanup function to
properly close the HTTP connection.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
write_events returns a list of Event structs, not a single Event. The typespec now correctly reflects response([Event.t()]). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns with Go client behavior where Recursive defaults to false (zero value). Users no longer need to explicitly set recursive when creating options structs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces ReadFromLatestEventOptions (read_everything | read_nothing) and ObserveFromLatestEventOptions (read_everything | wait_for_event) to prevent invalid combinations at the type level. Aligns with Go client which uses separate ReadFromLatestEvent and ObserveFromLatestEvent types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Renames predecessorhash to predecessor_hash, datacontenttype to data_content_type, and specversion to spec_version. Adds key mapping in Event.new/1 and ManagementEvent.new/1 to translate JSON field names to snake_case atoms. Updates all test assertions accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds "Aborting" sections to README and @doc annotations for read_events, observe_events, run_eventql_query, read_subjects, and read_event_types. Documents how to stop consuming lazy streams using Enum.take/2, stopping iteration, or terminating the consuming process. Aligns with Go client which documents context cancellation for the same operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_atom
Map.get evaluates the default value eagerly, causing
String.to_existing_atom("datacontenttype") to fail even when the key
exists in the mapping. Switch to Map.get_lazy which only evaluates
the fallback when the key is not found.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gossi
left a comment
gossi
left a comment
There was a problem hiding this comment.
Separating request options for read and observe from latest event is the only meaningful change in this PR.
One cosmetic one, that does extra work.
The stream handling notice is unnecessary (they better go). The error handling wrong at best.
| ### Aborting Reading | ||
|
|
||
| Since `read_events` returns a lazy stream, you can stop consuming it at any time. Use `Enum.take/2` to limit the number of events, or simply stop iterating. If you are consuming the stream in a separate process, you can abort by terminating that process. | ||
|
|
There was a problem hiding this comment.
that's implicit. And I consider it general knowledge about elixir when handling stream vs list. It's one of the first things you learn.
The comment is stating the obvious and isn't needed.
|
|
||
| *Note that `from_latest_event` and `lower_bound` can not be provided at the same time.* | ||
|
|
||
| ### Aborting Observing |
There was a problem hiding this comment.
same for all the others here...
|
|
||
| {:error, reason} -> | ||
| {:error, reason} | ||
| raise(reason) |
There was a problem hiding this comment.
this is no good here and unexpected. There is read_event_types() and read_event_types!() - the latter with the bang is expected to throw an arror, the former is expected to return the error.
Please don't throw in here, leave that to the bang versions of that functions (that's the reason for their existence).
|
|
||
| {:error, reason} -> | ||
| {:error, reason} | ||
| raise(reason) |
There was a problem hiding this comment.
Also in this case (and the one above). Is the proper return code to Req.parse_message() iirc.
| typedstruct do | ||
| field :data, any(), enforce: true | ||
| field :datacontenttype, String.t(), enforce: true | ||
| field :data_content_type, String.t(), enforce: true |
There was a problem hiding this comment.
I took rust impl as reference which keeps the name wo/ underscore. In the end eases handling those fields.
You can underscore name them to introduce extra work.
|
Closing in favor of a focused PR that only contains the FromLatestEventOptions split. |
2 participants
Summary
{:error, reason}returns inStream.resource/3withraise(reason)for proper exception propagationFromLatestEventOptionsintoReadFromLatestEventOptionsandObserveFromLatestEventOptionswith operation-specific valid valuespredecessor_hash,data_content_type,spec_version)write_eventsreturn type fromEvent.t()to[Event.t()]recursiveoption fromenforce: truetodefault: false, aligning with Go clientTest plan
FromLatestEventOptionssplit correctly prevents invalid combinations at type level🤖 Generated with Claude Code