feat: macOS all-timeout run yields a summary instead of throwing (#92)#94
Merged
Merged
Conversation
…mmary (#92) BSD ping exits 2 when a run draws ICMP errors back (TTL-exceeded) but no echo replies, and 1 on pure silence. PingMac mapped exit 1 -> noReply but left exit 2 unmapped, so the same logical no-reply outcome threw Exception('Ping process exited with code: 2') instead of terminating with a 100%-loss PingSummary, depending only on whether an intermediate hop returned a TTL-exceeded packet. Map exit 2 to PingError(noReply) alongside exit 1 in interpretExitCode and widen throwExit so 2 is no longer an unmapped throw. A recognized exit code short-circuits base_ping's unmapped-exit throw path, so the already-built 100%-loss summary becomes the terminal event with no synthetic error. Every genuinely-unmapped code still surfaces a catchable error then closes the stream (§spec:stream-lifecycle-robustness narrows by exactly one code). Add network-free (non-live) regression coverage in stream_lifecycle_test via a MacTestPing(PingMac) harness: exit 2 and exit 1 both yield identical 100%-loss summaries; unmapped exit 3 still throws-then-closes; clean zero-exit and unknownHost (68) runs are unchanged. Update platform_test exit-code unit assertions to the new mapping. Implements §spec:mac-all-timeout-summary.
…ce of truth)
Eliminates the duplicated {1,2,68} recognized-codes set in throwExit so it
cannot drift from interpretExitCode. Behavior-preserving (simplify gate).
Map macOS ping exit 2 to noReply so all-timeout runs always yield a 100%-loss PingSummary instead of a thrown exception. macOS-only, patch-level, no public API change. Includes non-live regression coverage.
Mirror the exit-2 assertions so the 'same 100%-loss summary shape' claim covers the count fields, not just packetLoss. Review finding (LOW).
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
1 participant
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.
What this builds
On macOS, BSD
pingexits with code2when a run draws ICMP error packets back but no echo replies (the classicttl=1/ TTL-exceeded case), and code1on pure silence.PingMacmapped exit1→noReplybut left exit2unmapped, so the same logical "no reply" outcome sometimes terminated with a clean 100%-lossPingSummary(exit 1) and sometimes surfacedException('Ping process exited with code: 2')(exit 2) — depending only on whether an intermediate hop happened to return a TTL-exceeded packet. A consumer awaitingstream.last/.drain()got the exception on the exit-2path even though a complete 100%-loss summary was already built.This reclassifies macOS exit
2as a recognizednoReplyoutcome, so an all-timeout macOS run always terminates with a deterministic 100%-loss summary, never a thrown exception.lib/src/ping/mac_ping.dart:interpretExitCodenow maps both exit1and exit2toPingError(noReply).throwExitis derived frominterpretExitCode(exitCode != 0 && interpretExitCode(exitCode) == null) sointerpretExitCodeis the single source of truth — the throw-list and no-reply-list cannot drift. Every code that is not a recognized no-reply/error code still surfaces a catchable exception, preserving the stream-lifecycle-robustness guarantee (it narrows by exactly one well-understood code, it does not loosen).errorslist carries the per-probe errors actually observed (requestTimedOut/timeToLiveExceeded) plus a single run-levelnoReply— identical in shape to the exit-1path, no synthetic/duplicate error.§spec: sections now complete
§spec:mac-all-timeout-summary— status flipped from not started → implemented inSPEC.md. Refines§spec:stream-lifecycle-robustnessby moving macOS exit2out of the "unmapped non-zero exit" bucket into a known outcome.Integration test path for the reviewer
The previously
live-only TTL-exceeded case is now covered deterministically (no live network, runs in the CI gate underdart test -x live) intest/stream_lifecycle_test.dart→ groupmacOS all-timeout summary (§spec:mac-all-timeout-summary):exit 2 (TTL-exceeded) yields a 100%-loss summary, never an exception— terminal event is aPingSummarywithtransmitted=2, received=0, packetLoss=100%;errors=[timeToLiveExceeded…, noReply]with exactly onenoReply; stream closes once; no exception on the error channel.exit 1 (pure silence) yields the same 100%-loss summary shape— determinism guard; pins the same count fields.regression: a genuinely-unmapped macOS exit still throws then closes(exit 3) — preserves robustness.regression:clean zero-exit run and recognized error exit (unknown host, 68) are unchanged.test/platform_test.dart:interpretExitCodemaps 1/2/68;throwExitignores 1/2/68 and throws for genuinely-unmapped codes.Run locally:
dart analyze --fatal-infos && dart test -x live(243 tests pass). Manual acceptance on macOS:Ping('201.202.203.204', count: 2, ttl: 1).stream.lastnow returns a 100%-loss summary instead of throwing.Review note
An automated code review was already completed in kandev during the Review step (see the task's plan artifact, "Code review — macOS all-timeout summary (#92)"): verdict clean — no correctness bugs, no spec violations; one LOW test-completeness finding was resolved (commit
1dc085f). CI core gate (analyze +dart test -x live) is green; the Swift/iOSxcodebuildjob runs on the GitHub macOS runner (not runnable on the Linux planning host; #92 touches no iOS/Swift code).