fix(event/stream): scale the close drain bound with PullTimeout

[ttradesman]

Scale the close drain bound with PullTimeout

Summary

closeDrainTimeout was a fixed 5 seconds. Close uses it to wait for the pull and renew goroutines to exit, but those loops block inside caller.SendSoap, which is not ctx-aware — as the caller interface documents, only the HTTP client's own timeout can unblock them. So the drain bound has to outlast an in-flight poll, and a fixed 5s only did that while PullTimeout was also 5s.

Why it matters

Any caller raising PullTimeout — which is the natural thing to do, since a long-poll is meant to be long and a longer poll means fewer requests — makes the drain the shorter of the two. Close then times out on every shutdown that lands mid-poll, which for a 30-second poll is most of them.

The consequence is not just a spurious error. When the drain times out, Close returns early and the Unsubscribe below it is skipped:

select {
case <-s.done:
case <-time.After(closeDrainTimeout):
    s.closeErr = fmt.Errorf("close: pull/renew loops did not drain within %s ...", closeDrainTimeout)
    return          // <- Unsubscribe never runs
}

So the pull-point is left registered at the camera until InitialTermination expires on its own. On a consumer that rebuilds its stream on reconfiguration or reconnect, that orphans a subscription per cycle, and the goroutine and its socket stay live until the HTTP call eventually returns.

Changes

The default is now derived rather than fixed, from whichever of the two relevant timeouts is longer:

d.CloseDrainTimeout = max(PullTimeout, dev.HttpClient.Timeout) + closeDrainSlack   // closeDrainSlack = 10s

Both matter, and taking only the first is not enough. PullTimeout is how long the camera holds a poll, but http.Client.Timeout is what actually caps the request — it has to cover dial, TLS and the response transfer on top of the poll it outlasts, so callers set it higher. A 30s poll behind a 40s ceiling would otherwise get a 40s drain against a call that can itself run 40s: a dead heat, and Close still loses the shutdowns that land mid-poll.

A zero client ceiling means unbounded, where no finite drain helps, so PullTimeout remains the best bound available in that case.

This is derived in NewStream, because that is the only entry point with the device in hand. Callers wiring their own caller implementation keep the PullTimeout-only bound. Options.CloseDrainTimeout overrides both, following the package's existing zero-means-default convention.

closeDrainTimeout the constant is replaced by closeDrainSlack, which is the part that was genuinely a constant; the bound itself was never one.

Tests

TestCloseDrainTimeout_ScalesWithPullTimeout covers the derivation from the default poll, derivation from a long poll, and an explicit value overriding both. It also asserts the invariant directly — the resolved drain must exceed the resolved PullTimeout — so a future change to either default cannot silently reintroduce the inversion.

TestDrainFor_BoundedByTheLongestPossibleCall covers the client-ceiling half: a ceiling above the poll becomes the bound, a ceiling below it does not, equal values leave the poll as the bound, and an unbounded ceiling falls back to the poll. Each case also asserts the resulting drain outlasts both inputs.

TestClose_BoundedWhenLoopsStuckOnHungHTTP now sets CloseDrainTimeout explicitly. It had been relying on the constant being small, and would otherwise sit for the derived default on every run.

Compatibility

Behavioural change for callers that set a PullTimeout above 5 seconds: Close now waits longer before giving up, which is the point — it waits long enough to drain cleanly and reach Unsubscribe. Callers who preferred the old fixed bound can set CloseDrainTimeout: 5 * time.Second and get exactly the previous behaviour. No signature changes; Options gains one optional field.

[cedricve]

@ttradesman we have some merge conflicts now, due to merging your previous PR's. All are currently deployed in v3.9.0. Many thanks!