fix: satisfy SDK compliance harness 0.8.0#200
Conversation
marandaneto
changed the title
posthog-ruby Compliance ReportDate: 2026-06-27 12:40:10 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
|
Reviews (1): Last reviewed commit: "chore: add SDK compliance harness 0.8.0" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Sends a request for the batch, returns [status_code, body] | ||
| def send_request(api_key, batch) | ||
| payload = JSON.generate(api_key: api_key, batch: batch) |
There was a problem hiding this comment.
@last_retry_after can leak into the next batch's first retry delay. When retries are exhausted after a 429 response that carries a Retry-After header, retry_delay_seconds is never called (because retries_remaining > 1 is false) and @last_retry_after retains the header value. On the next call to send, if @http.request raises an exception before the mutex block can overwrite the field, retry_delay_seconds reads the stale header and sleeps for that many seconds — potentially thousands of seconds — instead of using the backoff policy. Clearing the field unconditionally at the start of send_request closes the gap.
| # Sends a request for the batch, returns [status_code, body] | |
| def send_request(api_key, batch) | |
| payload = JSON.generate(api_key: api_key, batch: batch) | |
| # Sends a request for the batch, returns [status_code, body] | |
| def send_request(api_key, batch) | |
| @last_retry_after = nil | |
| payload = JSON.generate(api_key: api_key, batch: batch) |
| seconds = Float(value, exception: false) | ||
| return seconds if seconds&.positive? |
There was a problem hiding this comment.
Retry-After: 0 means "retry immediately", but 0.0.positive? returns false, so the method falls through to the HTTP-date branch, fails to parse "0", and returns nil. That causes the caller to use the backoff policy instead of obeying the header. Using seconds&.>=(0) matches the semantics of the header spec (any non-negative number is a valid delay, including zero).
| seconds = Float(value, exception: false) | |
| return seconds if seconds&.positive? | |
| seconds = Float(value, exception: false) | |
| return seconds if seconds&.>=(0) |
| it 'passes max_retries to the transport as total attempts' do | ||
| queue = Queue.new | ||
| worker = described_class.new(queue, 'secret', max_retries: 2) | ||
|
|
||
| expect(worker.instance_variable_get(:@transport_options)[:retries]).to eq(3) | ||
| end | ||
|
|
||
| it 'passes compression to the transport when enabled' do | ||
| queue = Queue.new | ||
| worker = described_class.new(queue, 'secret', enable_compression: true) | ||
|
|
||
| expect(worker.instance_variable_get(:@transport_options)[:gzip]).to eq(true) | ||
| end |
There was a problem hiding this comment.
max_retries: 0 path (documented as "disable retrying") is not exercised, so a regression in the + 1 calculation for the zero case would be invisible. Symmetrically, there is no test that enable_compression: false (or nil) leaves :gzip unset in @transport_options. The two new tests also duplicate the queue = Queue.new / described_class.new setup that could be shared in a single parameterised example group.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Problem
The SDK compliance workflow and local harness need to use SDK test harness release 0.8.0, with reusable GitHub workflow calls pinned to the release commit SHA instead of a mutable tag/branch. Running the updated harness exposed SDK/adapter compliance gaps in this repository.
Changes
Tests