Skip to content

Fix multi-log open race and debug log horizontal scrollbar#523

Merged
NikTilton merged 2 commits into
mainfrom
jschick/multi-log-and-debug-fixes
May 14, 2026
Merged

Fix multi-log open race and debug log horizontal scrollbar#523
NikTilton merged 2 commits into
mainfrom
jschick/multi-log-and-debug-fixes

Conversation

@jschick04
Copy link
Copy Markdown
Collaborator

Two unrelated runtime fixes off main.

Bugs

  • Multi-log empty table: opening a Live log immediately after another Live log left the table populated with the second log's rows briefly, then wiped to empty even though events had successfully arrived.
  • Debug log horizontal scrollbar: long debug log lines re-introduced a horizontal scrollbar in the Debug Log modal viewport.

Root cause

CloseAll race

Commit f046a07 flipped EventLogExpertEffects.HandleCloseAll's _logWatcherService.RemoveAll() (sync) to RemoveAllAsync() (async). Pre-f046a07 the entire effect body ran synchronously inside the Dispatch(CloseAllAction) frame; post-f046a07 the dispatches that follow the await race against HandleOpenLog. Fluxor's Dispatch runs reducers synchronously and starts effects but does not await them. Callers (MauiMenuActionService.OpenLogAsync, SettingsModal.ReloadOpenLogs) dispatch CloseAllAction immediately followed by OpenLogAction back-to-back synchronously. With Live then Live, a Channel watcher is registered, RemoveAllAsync returns a non-completed Task, the await yields, HandleOpenLog populates the table via AddTableAction + UpdateTableAction, and the continuation then dispatches LogTable.CloseAllAction which wipes the freshly populated table.

Debug log scrollbar

.debug-log-viewport { overflow-x: auto } combined with .debug-log-row { white-space: pre } on long lines (e.g., long stack traces or paths) re-introduced the horizontal scrollbar.

Fix

  • HandleCloseAll: dispatch LogTable.CloseAllAction / StatusBar.CloseAllAction and clear all in-memory bookkeeping (_resolverCache, _xmlResolver, _logsLoadedWithXml, _pendingSelectionRestore) BEFORE awaiting _logWatcherService.RemoveAllAsync(). The watcher drain is pure resource cleanup and does not read these caches, so flushing them up front is safe and ensures any caller-dispatched OpenLogAction runs against cleared state.
  • New regression test HandleCloseAll_DispatchesStateClearsBeforeWatcherDrain uses a TaskCompletionSource to hold RemoveAllAsync and asserts every state-clearing call already happened during the synchronous prefix.
  • Debug log modal: viewport overflow-x: hidden; rows get width: 100%, overflow: hidden, text-overflow: ellipsis (kept white-space: pre so leading indentation in stack traces still reads correctly). Each row gains a title="@line" attribute so users can hover to read the full clipped content as a native browser tooltip.
  • New regression test DebugLogModal_AfterLoad_RowsCarryTitleAttributeMirroringText pins the title-mirror invariant.

Verified

  • dotnet build EventLogExpert.slnx: 0 warnings / 0 errors.
  • dotnet format whitespace --verify-no-changes: clean.
  • dotnet format style --verify-no-changes --diagnostics IDE0005 IDE0065: clean.
  • dotnet test EventLogExpert.slnx --no-build: 1853 passed / 2 skipped / 0 failed across all 7 test projects.
  • Manually confirmed Live then Live no longer empties the second log.
  • Manually confirmed long debug-log lines now clip with ellipsis and reveal full content on hover.

Copilot AI review requested due to automatic review settings May 14, 2026 04:34
@jschick04 jschick04 requested a review from a team as a code owner May 14, 2026 04:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Addresses two runtime regressions: a Fluxor effect ordering race when opening multiple Live logs back-to-back (leading to a briefly-populated then emptied table), and a Debug Log modal horizontal scrollbar regression for long lines.

Changes:

  • Reordered Effects.HandleCloseAll so LogTable/StatusBar close actions and in-memory cache clears occur before awaiting watcher teardown, preventing OpenLogAction from racing into stale/soon-to-be-cleared UI state.
  • Added a regression unit test that holds RemoveAllAsync() via TaskCompletionSource to assert the synchronous “state-clearing prefix” happens before the awaited watcher drain.
  • Adjusted Debug Log modal rendering/CSS to clip long lines (no horizontal scrollbar) and added title mirroring for hover-to-see-full-text, with a new component test to pin the invariant.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Unit/EventLogExpert.UI.Tests/EventLog/EffectsTests.cs Adds regression coverage for HandleCloseAll ordering; minor cleanup to filter test helper usage; relocates an existing XML-reload test.
tests/Unit/EventLogExpert.Components.Tests/Modals/DebugLogModalTests.cs Adds regression test ensuring each rendered row mirrors its text into a title attribute.
src/EventLogExpert.UI/EventLog/Effects.cs Moves dispatches and cache clears ahead of awaiting watcher removal to eliminate the close/open race.
src/EventLogExpert.Components/Modals/DebugLogModal.razor.css Prevents horizontal scrollbar and applies clipping/ellipsis styling to long log rows.
src/EventLogExpert.Components/Modals/DebugLogModal.razor Adds title="@line" to rows; minor attribute ordering/class ordering adjustments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NikTilton NikTilton merged commit 28bb909 into main May 14, 2026
7 checks passed
@NikTilton NikTilton deleted the jschick/multi-log-and-debug-fixes branch May 14, 2026 17:56
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.

3 participants