diff --git a/event/stream/pulltimeout.go b/event/stream/pulltimeout.go new file mode 100644 index 0000000..82be75f --- /dev/null +++ b/event/stream/pulltimeout.go @@ -0,0 +1,42 @@ +package stream + +import ( + "fmt" + "time" + + "github.com/kerberos-io/onvif" +) + +// validateClientTimeout rejects an HTTP client ceiling that cannot +// outlast the PullMessages long-poll. +// +// PullMessages asks the camera to hold the connection open for up to +// PullTimeout. http.Client.Timeout bounds the entire exchange — dial, +// write, and the wait for response headers — and starts before the +// camera has parsed the request, so it always expires first when the +// two are equal. The pull then fails on every interval with no event, +// and the subscription survives only by being recreated after +// ReconnectAfterFailures, which replays the camera's whole property +// state each time. A zero client timeout means unbounded, which is safe +// here because the pull loop is already bounded by ctx. +func validateClientTimeout(clientTimeout, pullTimeout time.Duration) error { + if clientTimeout == 0 || clientTimeout > pullTimeout { + return nil + } + return fmt.Errorf( + "http.Client.Timeout (%s) must exceed PullTimeout (%s): PullMessages is a long-poll and the client would abort every quiet pull; raise the client timeout above PullTimeout or leave it zero", + clientTimeout, pullTimeout) +} + +// clientTimeoutOf reports the device's HTTP client ceiling, or 0 when +// the SDK is using its own default (unbounded) client. +func clientTimeoutOf(dev *onvif.Device) time.Duration { + if dev == nil { + return 0 + } + c := dev.GetDeviceParams().HttpClient + if c == nil { + return 0 + } + return c.Timeout +} diff --git a/event/stream/pulltimeout_test.go b/event/stream/pulltimeout_test.go new file mode 100644 index 0000000..4a40691 --- /dev/null +++ b/event/stream/pulltimeout_test.go @@ -0,0 +1,44 @@ +package stream + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestValidateClientTimeout — PullMessages is a long-poll: the camera +// holds the connection open for PullTimeout waiting for an event. An +// http.Client.Timeout covers the whole exchange and starts before the +// camera has even parsed the request, so a client ceiling at or below +// PullTimeout loses the race on every quiet interval and the pull can +// only ever fail. This shipped once (both were 5s) and presented as a +// slow camera rather than a misconfiguration. +func TestValidateClientTimeout(t *testing.T) { + tests := []struct { + name string + client time.Duration + pull time.Duration + wantErr bool + }{ + {"unbounded client is fine", 0, 30 * time.Second, false}, + {"comfortable headroom", 40 * time.Second, 30 * time.Second, false}, + {"strictly greater is accepted", 30*time.Second + time.Millisecond, 30 * time.Second, false}, + {"equal timeouts always lose", 5 * time.Second, 5 * time.Second, true}, + {"client below pull", 4 * time.Second, 30 * time.Second, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateClientTimeout(tt.client, tt.pull) + if tt.wantErr { + require.Error(t, err, "client=%s pull=%s must be rejected", tt.client, tt.pull) + assert.Contains(t, err.Error(), "PullTimeout", + "the error must name the option the caller has to change") + return + } + assert.NoError(t, err, "client=%s pull=%s must be accepted", tt.client, tt.pull) + }) + } +} diff --git a/event/stream/stream.go b/event/stream/stream.go index 9dac55b..0eb2760 100644 --- a/event/stream/stream.go +++ b/event/stream/stream.go @@ -234,6 +234,12 @@ func (s *Stream) updateGrantedTerminationIfGen(gen uint64, t time.Time) { // // The returned Stream stops when ctx is cancelled or Close is called. func NewStream(ctx context.Context, dev *onvif.Device, opts Options) (*Stream, error) { + // Checked before the subscription call: this config can only fail, + // so surfacing it here beats a stream that appears to work and + // silently survives on reconnects alone. + if err := validateClientTimeout(clientTimeoutOf(dev), opts.withDefaults().PullTimeout); err != nil { + return nil, err + } return newStream(ctx, deviceCaller{dev: dev}, opts) }