Retry transport-level send failures instead of dropping events

[sirsova-readdle]

Problem

Transient network failures cause Amplitude events to be silently dropped, raising a false alert each time. Observed in prod (piper, \$identify):

Failed to send event to Amplitude
error: HTTP request failed: Post "https://api2.amplitude.com/2/httpapi": read tcp ...: read: connection reset by peer

http.Client.Do returns a *url.Error for any request that never completed at the HTTP level (connection reset, EOF, refused, DNS, TLS). For a connection reset, Timeout() is false and there is no response, so responseStatus is unset. The switch in Process only routed to the retry path (processTimeout) for timeouts or 408/500, so a reset fell through to processUnknownError → all events go to the callback, none to retry → the plugin never calls storage.ReturnBack and the events are lost.

Fix

Broaden the retry branch so any transport-level failure (isURLErr) is retried with backoff, not only timeouts. A server-side HTTP error (4xx/5xx) returns a response and a nil error, so it is never a *url.Error; isURLErr == true reliably means "the request never reached a response" — exactly the class worth retrying. Retries are bounded by MaxRetries, after which events go to the callback as before, so nothing is dropped silently.

Note: net.Error.Temporary() is deliberately not used to narrow this — it is deprecated since Go 1.18 and, verified against the real error, returns false for read: connection reset by peer, so it would leave the bug unfixed.

Tests

Added TestTransportError_Retried: a *url.Error with Timeout() == false and no status (simulating connection reset) is routed to EventsForRetry with RetryCount incremented and RetryAt in the future. Existing timeout behavior is unchanged.

go test ./... and go vet ./... pass.

Ref: BKD-461

Follow-up (sparkback)

After this merges, sparkback's v2/go.mod replace github.com/amplitude/analytics-go => github.com/readdle/analytics-go v0.0.0-20231117134130-464d41bda376 must be re-pinned to the new commit hash to pick up the fix.

---

Note

Medium Risk
Changes event delivery failure handling for all transport errors; bounded by MaxRetries but broadens what gets retried (DNS, TLS, refused, etc.), which is intended but affects production analytics reliability.

Overview
Amplitude destination no longer treats non-timeout transport failures (e.g. connection reset by peer) as permanent errors that send all events to the callback.

The Process switch now routes any *url.Error from http.Client.Do into the same retry path as timeouts and 408/500, using existing MaxRetries and backoff via processTimeout. HTTP responses with status codes still classify by status; only failures before a response count as transport errors.

Adds TestTransportError_Retried with a simulated connection-reset *url.Error (Timeout() == false, no status) to assert events land in EventsForRetry with incremented RetryCount and scheduled RetryAt.

^{Reviewed by Cursor Bugbot for commit 90208fc. Bugbot is set up for automated code reviews on this repo. Configure here.}

[opodolian-readdle]

connection reset by peer happens while reading the response — the request was likely already ingested by Amplitude, so retrying re-sends those events. Dedup relies on InsertID (insert_id,omitempty); if the caller (piper $identify events) doesn't set it, this double-counts. Worth confirming InsertID is populated.

[sirsova-readdle]

InsertID is always populated: the caller passes empty, but the SDK's ContextPlugin (added by NewClient) assigns a UUID in the before-phase, before the event enters storage. Retries re-send the same stored StorageEvent (before-plugins don't re-run), so the InsertID is stable and Amplitude dedupes. This is also the exact mechanism the pre-existing timeout/500 retries already rely on — this PR only routes one more error class into it.