chore: optimize http timeout and flush interval for server-side usage#197
chore: optimize http timeout and flush interval for server-side usage#197duyhungtnn wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Optimizes default networking and event-flush settings intended for server-side usage of the SDK, aiming to reduce request latency on stalled connections and lower API load by batching events less frequently.
Changes:
- Increased default
eventsFlushIntervalfrom 10s to 30s. - Reduced HTTPS request timeout from 10s to 5s in the API client.
- Added explanatory comments in config validation around retry/deadline behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/config.ts |
Updates default flush interval and adds notes during flush-interval validation. |
src/api/client.ts |
Reduces HTTP request timeout used for API calls. |
src/__tests__/convert_config_to_bkt_config.ts |
Updates the default flush interval expectation in config-conversion tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Validate eventsFlushInterval | ||
| // | ||
| // Note: The SDK enforces a deadline of 80% of the flush interval for retry operations. | ||
| // Very short intervals (e.g., <20s) may limit the number of actual retry attempts | ||
| // if requests fail, even when MaxRetries is set to 3. However, failed events are | ||
| // automatically re-queued and will be retried in the next flush cycle. |
There was a problem hiding this comment.
The new note about an “80% of flush interval deadline”, “MaxRetries”, and failed events being automatically re-queued doesn’t appear to match the current implementation (I can’t find any retry/deadline logic, and scheduled flush removes events from the store before sending without re-adding them on failure). Please either implement the described behavior (with tests) or revise the comment to reflect what the SDK actually does today.
| // Validate eventsFlushInterval | |
| // | |
| // Note: The SDK enforces a deadline of 80% of the flush interval for retry operations. | |
| // Very short intervals (e.g., <20s) may limit the number of actual retry attempts | |
| // if requests fail, even when MaxRetries is set to 3. However, failed events are | |
| // automatically re-queued and will be retried in the next flush cycle. | |
| // Validate eventsFlushInterval. | |
| // If the configured value is below the minimum allowed, the SDK logs a warning | |
| // and falls back to the default flush interval. |
| authorization: this.apiKey, | ||
| }, | ||
| timeout: 10000, | ||
| timeout: 5000, | ||
| }; |
There was a problem hiding this comment.
With the shorter request timeout, it’s important that a timeout reliably rejects this Promise. Right now the 'timeout' handler calls clientReq.destroy() without rejecting (and destroy() without an error is not guaranteed to emit an 'error' event), which can leave the Promise pending indefinitely. Consider rejecting explicitly on timeout (or destroying with an error) so callers always get a failure signal.
Update DEFAULT_FLUSH_INTERVAL_MILLIS from 10_000 to 30_000 and revise comments to clarify that the minimum flush interval remains 10s while the default is 30s. This ensures the default flush cadence is less frequent than the enforced minimum and makes intent explicit in the code comments.
refs: bucketeer-io/go-server-sdk#190
Summary
Optimizes timeout and flush interval defaults for server-to-server communication.
Changes
HTTP timeout: 10s → 5s
Default flush interval: 10s → 30s
Added docs about retry behavior with custom intervals
5s timeout: Server-to-server requests are fast (RegisterEvents p95: 1-2s). With retry logic, fast failure is better than waiting 10s on stalled connections.
30s flush interval: Reduces API load 3× while maintaining proper retry coverage. Server-side SDKs run continuously (unlike browser SDKs where users close tabs), so 30s latency is acceptable for analytics.
Interaction: With 5s timeout + 30s flush (24s deadline), all 3 retry attempts fit: 5s + 1s + 5s + 2s + 5s = 18s ✅
Users can still configure shorter intervals if needed. Failed events are automatically re-queued.