[161] Format console errors and transaction tracking in network_sync_checker - #359
Open
kingeligma wants to merge 2 commits into
Conversation
…ifecycle - Add shared debug machinery mirroring the rabe_connector convention: formatStackTrace, formatConsoleWarningBlock, logNetworkSyncWarning, and a NetworkSyncTransactionTracker singleton. - Route all previously raw console.warn sites through formatted warning blocks (probe rejection, wallet availability, session persistence, stellar account extraction). - Track sync probe lifecycle in runNetworkSyncSign and validateNetworkSyncWithSignature via an optional txId; healthy approvals are recorded silently while failures emit one greppable TX <PHASE> block. - Add network_sync_checker_logging tests covering block formatting, warn emission counts per path, and tracker history filtering+clearing. Also carries the suite-repair changes (SignatureTimeoutAlert useMemo, localStorage shim, freighter_m instance XDR fixtures, useWallet stubs) so CI runs green on every derived branch. Closes 161
There was a problem hiding this comment.
Pull request overview
This PR standardizes network_sync_checker console warnings into consistent, greppable blocks (including stack traces) and introduces a lightweight transaction lifecycle tracker to correlate probe phases via optional txIds. It also includes test suite repairs and new tests to validate the logging/tracking behavior.
Changes:
- Added
formatStackTrace,formatConsoleWarningBlock, andlogNetworkSyncWarning, and replaced ad-hocconsole.warncalls with the formatted warning blocks. - Introduced
NetworkSyncTransactionTracker+networkSyncTrackersingleton, and extendedrunNetworkSyncSign/validateNetworkSyncWithSignatureto optionally track viatxId. - Added/updated Vitest + RTL tests and setup shims (including a
localStoragefallback invitest.setup.ts) to keep CI green across newer Node runtimes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.setup.ts |
Adds a defensive localStorage shim for jsdom tests when Node exposes an undefined experimental localStorage global. |
app/lib/network_sync_checker.ts |
Implements formatted warning blocks + stack formatting, adds tx lifecycle tracking, wires tracking into signature probe helpers, and updates warning logging sites. |
app/components/SignatureTimeoutAlert.tsx |
Replaces effect-driven parse error state with memoized derived state to avoid cascading renders. |
__tests__/signature_timeout_alert.test.tsx |
Stubs useWallet to avoid Node-ESM import issues and wraps wallet state mutations with act(). |
__tests__/network_sync_checker_logging.test.ts |
New tests covering block rendering, warn emission contracts, and tracker history/clear behavior. |
__tests__/freighter_multisig_hook.test.ts |
Replaces arbitrary base64 fixtures with real signed envelopes to exercise the real XDR parser paths. |
__tests__/freighter_connector.component.test.tsx |
Stubs useWallet to keep freighter connector logic under test without importing WalletProvider dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| }; | ||
| } | ||
| return { | ||
| return { |
Comment on lines
+129
to
+135
| const entry: NetworkSyncTxTrackEntry = { | ||
| txId, | ||
| phase, | ||
| message, | ||
| timestamp: Date.now(), | ||
| stack: formatStackTrace(err), | ||
| }; |
|
@kingeligma Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…cing timers Three timeout tests attached promise.catch() only after awaiting timer advance, so the rejected outer promise could be snapshotted as unhandled by Node during the interleaved microtask gap, making vitest report 3 unhandled errors and exit non-zero despite all tests passing. Attach the handler immediately after runSignatureWithTimeout (mirroring the .rejects-based tests) to keep npm run test exiting cleanly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #161
What
Formats
network_sync_checkerconsole output into consistent, greppable warning blocks and adds transaction lifecycle tracking, mirroring therabe_connector/ledger_usb_bridgeconvention:formatStackTrace,formatConsoleWarningBlock,logNetworkSyncWarningNetworkSyncTransactionTracker(track / getHistory / clear) +networkSyncTrackersingletonconsole.warnsites now emit a bordered, prefixed block with a stack tracerunNetworkSyncSign/validateNetworkSyncWithSignatureaccept an optionaltxIdand record probe lifecycle; healthy approvals are tracked silently, while rejections/mismatches emit exactly oneTX <PHASE>blockTests
__tests__/network_sync_checker_logging.test.tscovers block rendering, single-warn emission per path (success = silent, rejection = oneTX ERRORblock, matches the existing warn-count contracts), and tracker history filtering + clearing.Suite repair (carried to every derived branch)
The base suite is broken on
main(red CI). This branch also carries the fixes verified on the #162 branch so checks are green here too:SignatureTimeoutAlertuseMemo (fixes eslintreact-hooks/set-state-in-effect)vitest.setup.tslocalStorage shim for Node 24+/26 experimental globalsfreighter_multisig_hooktestsuseWalletstubs +act()fixes in the freighter component / signature-timeout suitesChecks
npm run test(87 files / 1328 tests),next build— all pass