Skip to content

move batch resolve new repo - #2

Merged
vahidlazio merged 2 commits into
mainfrom
batch-resolve-new
Aug 28, 2025
Merged

move batch resolve new repo#2
vahidlazio merged 2 commits into
mainfrom
batch-resolve-new

Conversation

@vahidlazio

Copy link
Copy Markdown
Collaborator

No description provided.

@vahidlazio
vahidlazio merged commit f2a0fac into main Aug 28, 2025
5 checks passed
@vahidlazio
vahidlazio deleted the batch-resolve-new branch August 28, 2025 09:48
vahidlazio added a commit that referenced this pull request Aug 24, 2026
Java (#2, latent trap): consumeVoidResponse / consumeTypedResponse fed addr
straight into consume(), which reads the length prefix at addr-4. On a null
response that reads 0xFFFFFFFC and traps or returns garbage. Go already guarded
this; Java now does too, and flushEvents normalizes the null to an empty
response so it cannot NPE a caller.

JS (#3, #8, silent failure): EventWasmResolver caught every error but only acted
on WebAssembly.RuntimeError, so a proto failure or guest-reported error was
swallowed with no log and no rethrow — and flushEvents returned an empty batch,
making a failure indistinguishable from a genuine empty flush. trackEvent now
rethrows non-trap errors (the provider's track() logs them) and flushEvents logs
before returning empty. Two regression tests added.

Rust (#4, #5): dropped the unsafe unwrap_unchecked, which was inherited from
AssignLogger where clippy::unwrap_used is denied — this crate has no such lint,
so a plain pop_front() match is both safe and clearer. Replaced the
precedence-sensitive `a <= b || c == 0 && d == 0` with a named
is_lone_oversized_event condition.

Rust (#9): added a concurrency test. The design claims track() is lock-free and
safe alongside a flush holding the state mutex, but every test was
single-threaded. 4 producers x 500 events against a concurrent drain, asserting
no loss and no duplication.

Documentation (#1, #6): added confidence-event-engine/README.md covering the
payload merge and key-collision rules, the at-most-once delivery guarantee with
its retry/observability mitigations and the cases where events are still lost,
and the Go value:0 limitation. These were only in code comments before.

Pushed back on two findings, with reasoning in the README and code:
- #1: Go genuinely cannot distinguish value 0 from unset (plain float64, no set
  flag). Always sending would attach a spurious value: 0 to every event where
  the caller set none — more common than deliberately tracking zero — so
  treating 0 as unset is the lesser divergence. Documented rather than changed.
- #7: Python's async publish is not a loss vector. drain returns on an empty
  WASM buffer, then shutdown(wait=True) waits for every submitted publish before
  the channel closes, and failures go through the same counter Go and Java use.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vahidlazio added a commit that referenced this pull request Sep 3, 2026
#1 Python flush counter race: protect _flush_succeeded/_flush_failed
   with _event_stats_lock (read, drain, restore all under lock).

#2 Cloudflare cross-queue KV race: merge update_prometheus_kv and
   update_events_kv into single update_kv_snapshot — one read-modify-
   write per call, no cross-queue clobber.

#3 Java silent exception: add logger.warn in empty-destinations catch.

#4 Go drain/restore duplication: extract TelemetryCounters struct with
   DrainAndStamp, RestoreOnFailure, RecordEventBatch. Both grpc.go and
   multi.go embed it — ~47 lines of duplication removed.

#5 JS double decode/encode: merge addProviderInitTelemetry and
   addFlushDeliveryTelemetry into single enrichTelemetry method —
   one decode/encode pass on first flush.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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