Skip to content

chore: clear shutdown noise and compiler warnings - #12

Merged
yordis merged 4 commits into
mainfrom
yordis/chore-clear-shutdown-noise-and-warnings
Sep 14, 2026
Merged

chore: clear shutdown noise and compiler warnings#12
yordis merged 4 commits into
mainfrom
yordis/chore-clear-shutdown-noise-and-warnings

Conversation

@yordis

@yordis yordis commented Sep 14, 2026

Copy link
Copy Markdown
Member
  • Stopping the application logged connection errors that looked like failures but were only the advisory lock process being killed before it could check its connection back in, and noise that is expected trains people to ignore logs
  • Newer Elixir versions warn about requires that are never used, and warnings we tolerate hide the ones we should act on

Cherry-picked from commanded/eventstore#317 (d4cc6c3, d7ce76d, FinOpsTech Bot and Silvan Büdenbender) and commanded/eventstore#326 (364bf7d, a9f3b6e, @wimwian).

wimwian and others added 4 commits September 14, 2026 18:17
@wimwian @yordis
fix: remove unused require Logger
690e86d 
EventStore.Storage.Lock and EventStore.Storage.Database never call any
Logger.* function, so `require Logger` triggers an "unused require
Logger" warning on Elixir 1.20. Removing it since it has no effect on
behavior.

(cherry picked from commit 364bf7d228be2cd5752f8b374eadaf23711bc289)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@wimwian @yordis
fix: pattern-match RecordedEvent struct in prepare_events/4
7a01cde 
Elixir 1.20's type checker can't prove `recorded_event` (destructured
from the {recorded_event, index} tuple produced by Enum.with_index/2)
is a %RecordedEvent{} at the point it's used in a struct update:

    warning: a struct for EventStore.RecordedEvent is expected on
    struct update

map_to_recorded_event/3 always returns %RecordedEvent{}, so this is a
type-checker visibility gap, not a real bug — pattern-matching
%RecordedEvent{} in the anonymous function head makes the invariant
explicit and resolves the warning.

(cherry picked from commit a9f3b6e360d9ed7a3f3e9ae9486a88ec2bec4923)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@FOT-BOT @yordis
Trap exit for graceful shutdown
7d10c6e 
In our testsuit we observed the following logs producing noise: `[info] Postgrex.Protocol (#PID<0.867.0>) disconnected: ** (DBConnection.ConnectionError) client #PID<0.868.0> exited`.

They seem to be cause by calling `Application.stop(:eventstore)` at the end of our test case. The root cause appears to be the hard kill that `AdvisoryLocks` experiences without the ability to checkin a currently held `DBConnection`. Trapping exits is sufficient to allow for a normal connection checkin before termination.

(cherry picked from commit d4cc6c3fec8c938965f82613e3e63ec307a40bb3)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Silvan Büdenbender @yordis
Handle EXIT message from non-parent processes
0cc11a9 
(cherry picked from commit d7ce76d46facb372ac057bb0fa74b3c7c5b1b8c1)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Mostly logging and compile-time fixes; advisory lock exit handling is a small lifecycle tweak that still stops the process on abnormal exits.

Overview
Advisory lock shutdown is handled more cleanly: EventStore.AdvisoryLocks now traps exits and treats normal {:EXIT, _, :normal} as no-ops while stopping the GenServer on other exit reasons. That avoids misleading connection/check-in errors when the app stops and the lock process is torn down before the DB connection is returned.

Compiler hygiene: unused require Logger is removed from EventStore.Storage.Database and EventStore.Storage.Lock.

Stream append typing: prepare_events/4 in EventStore.Streams.Stream pattern-matches %RecordedEvent{} explicitly in the enum step, clearing the unused-variable / typing warning without changing append behavior.

Reviewed by Cursor Bugbot for commit 0cc11a9. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 3 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b95ffb99-ffa1-42a6-96c7-e6e9aca8387a

📥 Commits

Reviewing files that changed from the base of the PR and between f6a1fc6 and 0cc11a9.

📒 Files selected for processing (4)
  • lib/event_store/advisory_locks.ex
  • lib/event_store/storage/database.ex
  • lib/event_store/storage/lock.ex
  • lib/event_store/streams/stream.ex

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yordis
yordis merged commit 01aea6a into main Sep 14, 2026
11 of 13 checks passed
@yordis
yordis deleted the yordis/chore-clear-shutdown-noise-and-warnings branch September 14, 2026 23:31
@sht-bot sht-bot mentioned this pull request Sep 15, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yordis @wimwian @FOT-BOT