Skip to content

Conversation

renatillas
Copy link
Owner

No description provided.

Copy link

rover-app bot commented Aug 20, 2025

Initiating social contact...

We're scanning your PR for issues. Stand by for comments.

Lots of love,
Rover 🤖

Copy link

@rover-app rover-app bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rover alert scan for re.natillas

Scanned on Aug 20th 2025, 10:28AM for commit d09eabaf5eb8d310ce95a7200e31a81d03a56205

Rover found 2 risks.
1 high, 1 medium.

The PR introduces silent error masking in actor communication and unhandled initialization failures, potentially leading to data corruption and application crashes.

To request another review, comment @rover-app review in the PR discussion.

Rover Support

Rover has scanned for issues in performance, security, reliability that might be introduced by this PR, in the context of your upstream and downstream services and dependencies.

What happens next

You can re-request a review by commenting @rover-app review on the PR.
Rover will review the PR again, and close any alerts that you've fixed.

I want to follow up with Rover

PR chat is coming

Soon, you'll be able to talk to Rover about issues in your PR, in your PR.
Right now, we only support code chat on your `main`/`master` (default) branch:
head to the graph page on the Rover platform
to chat with your code.

If Rover isn't doing much

It could be that Rover doesn't support your language or framework yet, or perhaps you've found an area we can improve in!
We'd love to get your feedback to help improve Rover, so if you're not happy with its output please get in touch by clicking here.

I love/hate the alerts Rover is generating

Regardless, we'd love to hear it!
We're working hard to make Rover better,
so please get in touch with us
with your PR number and alert comment.

I'd like to request a feature or improvement

You know the score: get in touch!
We love to have feature requests from our users to work on.

Rover actions

Re-review

Comment @rover-app review on the PR to request another review.

Suspend Rover scanning

To stop Rover from scanning PRs on your org (re.natillas), head to your organization settings or suspend the GitHub app installation on this GitHub account.

@renatillas
Copy link
Owner Author

@rover-app review

Copy link

rover-app bot commented Aug 20, 2025

Hey @renatillas!

We've received your review request, and we're processing it now!

That's all, folks!
Rover 🤖

Copy link

@rover-app rover-app bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rover alert scan for re.natillas

Scanned on Aug 20th 2025, 03:52PM for commit 00ba078a63d9554f08912f12b91f8bbd5e943b87

Rover found 3 risks.
1 high, 2 medium.

This PR introduces potential data loss and error handling issues in the event store, particularly around actor failures and snapshot management. The changes could lead to silent data corruption and application instability.

Other Issues

We could not match the following issues to corresponding files within the code changes

The new error handling in `load_events` causes `commit_events` to potentially overwrite existing events with only new events if there's a temporary actor failure.

Severity high

Description The new error handling in `load_events` causes `commit_events` to potentially overwrite existing events with only new events if there's a temporary actor failure.

Suggested fix

fn commit_events(memory_store: MemoryStore(entity, command, event, error), aggregate: Aggregate(entity, command, event, error), events: List(event), metadata: List(#(String, String))) -> Result(#(List(EventEnvelop(event)), Int), EventSourcingError(error)) {
  let Aggregate(aggregate_id, _, sequence) = aggregate
  let wrapped_events = wrap_events(aggregate_id, sequence, events, metadata)
  use past_events_result <- load_events(memory_store, Nil, aggregate_id, 0)

  case past_events_result {
    Ok(past_events) -> {
      let all_events = list.append(past_events, wrapped_events)
      actor.send(memory_store.events_subject, SetEvents(aggregate_id, all_events))
      let assert Ok(last_event) = list.last(wrapped_events)
      Ok(#(wrapped_events, last_event.sequence))
    }
    Error(err) -> {
      Error(err)
    }
  }
}

`save_snapshot` doesn't detect or propagate errors when sending snapshots to the actor, leading to potential silent snapshot loss.

Severity medium

Description `save_snapshot` doesn't detect or propagate errors when sending snapshots to the actor, leading to potential silent snapshot loss.

Suggested fix

fn save_snapshot(
  memory_store: MemoryStore(entity, command, event, error),
  snapshot: eventsourcing.Snapshot(entity),
) -> Result(Nil, EventSourcingError(error)) {
  case actor.send(memory_store.snapshot_subject, SetSnapshot(snapshot.aggregate_id, snapshot)) {
    Ok(_) -> Ok(Nil)
    Error(_) -> Error(eventsourcing.UnexpectedError)
  }
}

`load_snapshot` treats actor communication failures as if no snapshot exists, causing potential data inconsistency when the system makes decisions based on this misleading information.

Severity medium

Description `load_snapshot` treats actor communication failures as if no snapshot exists, causing potential data inconsistency when the system makes decisions based on this misleading information.

Suggested fix

fn load_snapshot(
  memory_store: MemoryStore(entity, command, event, error),
  aggregate_id: eventsourcing.AggregateId,
) -> Result(
  Option(eventsourcing.Snapshot(entity)),
  eventsourcing.EventSourcingError(error),
) {
  case process.call(memory_store.snapshot_subject, load_snapshot_time, GetSnapshot(
    aggregate_id,
    _,
  )) {
    Some(snapshot) -> Ok(Some(snapshot))
    None -> Ok(None)
    Error(_) -> Ok(None)
  }
}

To request another review, comment @rover-app review in the PR discussion.

Rover Support

Rover has scanned for issues in performance, security, reliability that might be introduced by this PR, in the context of your upstream and downstream services and dependencies.

What happens next

You can re-request a review by commenting @rover-app review on the PR.
Rover will review the PR again, and close any alerts that you've fixed.

I want to follow up with Rover

PR chat is coming

Soon, you'll be able to talk to Rover about issues in your PR, in your PR.
Right now, we only support code chat on your `main`/`master` (default) branch:
head to the graph page on the Rover platform
to chat with your code.

If Rover isn't doing much

It could be that Rover doesn't support your language or framework yet, or perhaps you've found an area we can improve in!
We'd love to get your feedback to help improve Rover, so if you're not happy with its output please get in touch by clicking here.

I love/hate the alerts Rover is generating

Regardless, we'd love to hear it!
We're working hard to make Rover better,
so please get in touch with us
with your PR number and alert comment.

I'd like to request a feature or improvement

You know the score: get in touch!
We love to have feature requests from our users to work on.

Rover actions

Re-review

Comment @rover-app review on the PR to request another review.

Suspend Rover scanning

To stop Rover from scanning PRs on your org (re.natillas), head to your organization settings or suspend the GitHub app installation on this GitHub account.

@renatillas renatillas merged commit eb6e768 into main Aug 20, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant