Skip to content

Reduce non-exception logging noise#42

Open
bsquidwrd wants to merge 6 commits intomainfrom
claude/improve-error-handling-7OSpy
Open

Reduce non-exception logging noise#42
bsquidwrd wants to merge 6 commits intomainfrom
claude/improve-error-handling-7OSpy

Conversation

@bsquidwrd
Copy link
Copy Markdown
Owner

No description provided.

- Fix infinite spin-loops in ObtainLock and WaitForAuthUnlockAsync by
  adding exponential backoff and deadline timeouts to prevent CPU spin
  and application hangs when Redis is slow or locks are contended
- Wrap async void Twitch monitor event handlers (Online/Update/Offline)
  in try-catch to prevent unhandled exceptions from crashing the process
- Guard GetAuthAsync against NullReferenceException when no active auth
  entry exists in the database; throws with a clear diagnostic message
- Fix AddOrUpdateAsync deadlock: wrap the update path in try/finally so
  the SemaphoreSlim is always released even when SaveChanges throws
- Guard RemoveAsync against NullReferenceException when the entity is
  not found by FindAsync
- Add null check for streamUser in StreamUpdateConsumer before accessing
  Username and other properties
- Add null check for monitorUser in MonitorModule.GetStreamUserAsync
  and throw ArgumentException with a user-visible message
- Fix GuildUpdated NullReferenceException by switching from instance
  .Equals() to static string.Equals() for nullable IconUrl comparison
- Add per-guild try-catch in DiscordMemberLiveConsumer so one failing
  guild does not prevent notifications for all remaining mutual guilds
- Dispose CancellationTokenSource in SendDiscordMessage using 'using'
  to prevent a resource leak on every notification send
- Replace bare catch{} blocks in InteractionHandler with typed catches
  that log warnings/errors for better observability

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
The streamUser null check was placed after SubscriptionRepository.FindAsync
which already passed streamUser as a query parameter - a null streamUser
would cause EF to throw or return unexpected results.

Also add a null guard for `user` itself, since GetUserById can return null
when the Twitch API does not find the account.

Reorder to: resolve user -> guard null user -> resolve streamUser ->
guard null streamUser -> query subscriptions.

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
…g error isolation

- Fix ConsolidateRoleMentions: List.ForEach with async delegate is
  effectively async void - exceptions silently swallowed and removes
  not awaited. Replace with a proper await foreach loop.
- Fix GetSubscriptionEmbed: guild.GetRole() returns null for deleted
  roles. Calling .Name on the result and OrderBy(i => i.Name) would
  NRE. Filter null roles out after the Select.
- Fix EscapeSpecialDiscordCharacters: Format.Sanitize throws on null
  input. Guard with null/empty check returning string.Empty instead.
- Fix GetStreamEmbed: game.Name and stream.StreamURL can be null/empty.
  Discord.NET throws ArgumentException on empty embed field values.
  Fall back to '[Not Set]' and user.ProfileURL respectively.
- Add per-item try-catch in DiscordGuildDeleteConsumer subscription
  loop and channel loop so one failed removal does not abort cleanup
  for all remaining subscriptions/channels.
- Add per-item try-catch in DiscordChannelDeleteConsumer subscription
  loop for the same reason.
- Guard Queues.QueueURL, QueueUsername, and QueuePassword against null
  env vars. Previously they would silently pass null to MassTransit,
  producing cryptic connection/auth failures at runtime.

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
…v reads

StreamUpdateConsumer:
- Move subscriptions check before game-related DB queries so that if a
  user has no subscriptions, the game lookup and potential AddOrUpdate
  are skipped entirely — saves 1-2 DB round-trips per event when there
  are no active subscriptions.
- Remove the second SingleOrDefaultAsync call after AddOrUpdateAsync in
  the else branch. The returned game entity was assigned to a local
  variable that was never read again in the method, making it a
  completely wasted DB query on every stream-update event.

DiscordMemberLiveConsumer:
- Replace List<SocketGuild> + Any(i => i.Id == guild.Id) deduplication
  with a HashSet<ulong>. The previous approach was O(n) per guild
  inside a nested shard/guild loop — O(n²) overall. HashSet.Add()
  returns false for duplicates, making the dedup O(1) per guild.

TwitchMonitor.UpdateUsers:
- Remove redundant ContainsKey + [key]=value / else TryAdd branching on
  ConcurrentDictionary. The indexer assignment (dict[key] = value)
  already handles both add and update atomically, eliminating an extra
  dictionary lookup per user on every user-refresh cycle.

Queues:
- Cache the RabbitMQ host string in a private static readonly field
  instead of calling Environment.GetEnvironmentVariable on every
  QueueURL property access.

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
…ync void

Infinite recursion in all API_Get* credential error handlers:
Every method (API_GetGame, API_GetUserByLogin, API_GetUserById,
API_GetUsersById, API_GetUserByURL, API_GetStream) caught
InvalidCredentialException/BadScopeException and called itself
without passing the retryCount through. A persistent auth failure
would recurse indefinitely until stack overflow. Each credential
catch block now propagates retryCount and returns null once
ApiRetryCount is exhausted, matching the existing BadGateway behaviour.

SetupAuthTimer - two issues fixed together:
1. A new System.Timers.Timer was allocated on every call (every
   auth refresh cycle), with the old one left for GC. The timer is
   now created once and reused; subsequent calls simply update the
   Interval and restart it.
2. The Elapsed handler used `async (sender, e) => await UpdateAuth()`
   which is async void — an unhandled exception from UpdateAuth would
   propagate to the thread pool and terminate the process. Replaced
   with a fire-and-forget pattern using ContinueWith(OnlyOnFaulted)
   to log any unhandled faults without crashing.

UpdateAuth - oldAuth expiry uses UpdateAsync instead of AddOrUpdateAsync:
The old token was fetched from the DB moments before. Using
AddOrUpdateAsync to mark it expired searches for a match and would
insert a duplicate row if the token somehow wasn't found.
UpdateAsync is semantically correct and avoids the unnecessary upsert
predicate evaluation.

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
@bsquidwrd bsquidwrd force-pushed the claude/improve-error-handling-7OSpy branch from 57a10b4 to 25dfc76 Compare February 19, 2026 04:22
… Info logs

DatabaseSetup.cs (Twitch Watcher) hardcoded MinimumLevel.Information()
regardless of the IsDebug flag, while LiveBotSetupHelpers.cs (Discord)
correctly switches to LogEventLevel.Debug when IsDebug=true. This meant
debug-mode logging was silently disabled for the entire Twitch Watcher
service. Both services now use the same conditional:
  IsDebug ? LogEventLevel.Debug : LogEventLevel.Information

Serilog replaces the .NET logging pipeline entirely, so the Logging
section in appsettings.json has no effect — the Serilog MinimumLevel
is the only gate that matters.

StreamOfflineConsumer: "No subscriptions found" was LogInformation.
This fires for every stream-offline event where the user has no LiveBot
subscription, which is the common case for most monitored streams.
Downgraded to LogDebug.

TwitchMonitor.UpdateAuth: "Updating Auth" and "Successfully set
AccessToken" were both LogInformation. These fire on every scheduled
auth refresh cycle and on every InvalidCredentialException retry (now
up to ApiRetryCount times). Downgraded to LogDebug so they only surface
when IsDebug=true.

https://claude.ai/code/session_01DGjiqffs4cNf5LX7A3YnLx
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.

2 participants