feat: SDK Compliance#2847
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 No Changeset FoundThis PR doesn't include a changeset. A changeset (and the release label) is required to release a new version. How to add a changesetRun this command and follow the prompts: pnpm changesetRemember: Never use |
|
Size Change: 0 B Total Size: 5.88 MB ℹ️ View Unchanged
|
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
WIP |
d3e39be to
4310f51
Compare
4310f51 to
a29f437
Compare
posthog-node Compliance ReportDate: 2026-01-21 11:57:52 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 91ms |
| Format Validation.Event Has Uuid | ✅ | 14ms |
| Format Validation.Event Has Lib Properties | ✅ | 12ms |
| Format Validation.Distinct Id Is String | ✅ | 16ms |
| Format Validation.Token Is Present | ✅ | 17ms |
| Retry Behavior.Retries On 503 | ✅ | 11023ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 5021ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 5019ms |
| Retry Behavior.Respects Retry After Header | ✅ | 8019ms |
| Retry Behavior.Implements Backoff | ✅ | 24030ms |
| Deduplication.Generates Unique Uuids | ✅ | 20ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 8020ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 14ms |
| Compression.Sends Gzip When Enabled | ✅ | 7ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 9ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2011ms |
| Error Handling.Retries On 408 | ✅ | 8018ms |
Failures
retry_behavior.does_not_retry_on_400
Expected 1 requests, got 2
retry_behavior.does_not_retry_on_401
Expected 1 requests, got 2
posthog-js Compliance ReportDate: 2026-01-21 12:04:01 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ❌ | 2152ms |
| Format Validation.Event Has Uuid | ✅ | 2019ms |
| Format Validation.Event Has Lib Properties | ✅ | 2014ms |
| Format Validation.Distinct Id Is String | ❌ | 2016ms |
| Format Validation.Token Is Present | ❌ | 2015ms |
| Retry Behavior.Retries On 503 | ❌ | 7018ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 4021ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 4018ms |
| Retry Behavior.Respects Retry After Header | ❌ | 7022ms |
| Retry Behavior.Implements Backoff | ✅ | 17031ms |
| Deduplication.Generates Unique Uuids | ✅ | 2025ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 7019ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 2015ms |
| Compression.Sends Gzip When Enabled | ❌ | 2013ms |
| Batch Format.Uses Proper Batch Structure | ❌ | 2012ms |
| Error Handling.Does Not Retry On 413 | ✅ | 4016ms |
| Error Handling.Retries On 408 | ❌ | 7017ms |
Failures
format_validation.event_has_required_fields
Expected distinct_id='test_user', got 'None'
format_validation.distinct_id_is_string
Expected distinct_id='test_user_123', got 'None'
format_validation.token_is_present
'list' object has no attribute 'get'
retry_behavior.retries_on_503
Expected at least 3 requests, got 2
retry_behavior.respects_retry_after_header
Retry delay too short: 44ms < 2500ms
compression.sends_gzip_when_enabled
Header 'Content-Encoding' with value 'gzip' not found in requests
batch_format.uses_proper_batch_structure
Batch format missing 'api_key' field at root level
error_handling.retries_on_408
Expected at least 2 requests, got 1
|
|
||
| const { distinct_id, event, properties } = req.body | ||
|
|
There was a problem hiding this comment.
register() sets persistent properties across all events, not the distinct_id for a single capture. Use identify() to set distinct_id, or pass it directly in the capture call via $distinct_id in properties.
| const { distinct_id, event, properties } = req.body | |
| // Identify the user if distinct_id provided | |
| if (distinct_id) { | |
| state.instance.identify(distinct_id) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/sdk_compliance_adapter/adapter.js
Line: 289:291
Comment:
`register()` sets persistent properties across all events, not the `distinct_id` for a single capture. Use `identify()` to set distinct_id, or pass it directly in the capture call via `$distinct_id` in properties.
```suggestion
// Identify the user if distinct_id provided
if (distinct_id) {
state.instance.identify(distinct_id)
}
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| try { | ||
| // Set distinct_id if provided | ||
| if (distinct_id) { | ||
| state.posthog.register({ distinct_id }) |
There was a problem hiding this comment.
register() sets persistent properties across all events, not the distinct_id for a single capture. Use identify() to set distinct_id, or pass it directly in the capture call.
| try { | |
| // Set distinct_id if provided | |
| if (distinct_id) { | |
| state.posthog.register({ distinct_id }) | |
| // Set distinct_id if provided | |
| if (distinct_id) { | |
| state.posthog.identify(distinct_id) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/sdk_compliance_adapter/adapter.ts
Line: 130:134
Comment:
`register()` sets persistent properties across all events, not the `distinct_id` for a single capture. Use `identify()` to set `distinct_id`, or pass it directly in the capture call.
```suggestion
// Set distinct_id if provided
if (distinct_id) {
state.posthog.identify(distinct_id)
}
```
How can I resolve this? If you propose a fix, please make it concise.| - test-network | ||
|
|
||
| test-harness: | ||
| image: posthog-sdk-test-harness:debug |
There was a problem hiding this comment.
Hardcoded image posthog-sdk-test-harness:debug requires manual build or pull. Consider adding build instructions to README or using a published image tag.
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/sdk_compliance_adapter/docker-compose.yml
Line: 12:12
Comment:
Hardcoded image `posthog-sdk-test-harness:debug` requires manual build or pull. Consider adding build instructions to README or using a published image tag.
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| - test-network | ||
|
|
||
| test-harness: | ||
| image: posthog-sdk-test-harness:debug |
There was a problem hiding this comment.
Hardcoded image posthog-sdk-test-harness:debug requires manual build or pull. Consider adding build instructions to README or using a published image tag.
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/node/sdk_compliance_adapter/docker-compose.yml
Line: 12:12
Comment:
Hardcoded image `posthog-sdk-test-harness:debug` requires manual build or pull. Consider adding build instructions to README or using a published image tag.
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.| const lastEvent = state.capturedEvents[state.capturedEvents.length - 1] | ||
|
|
||
| res.json({ success: true, uuid: lastEvent?.uuid || 'generated-uuid' }) |
There was a problem hiding this comment.
2-second hardcoded delay may be insufficient for high-latency networks or cause slow tests. Consider making this configurable via environment variable or using a retry/polling strategy to check if events were sent.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/sdk_compliance_adapter/adapter.js
Line: 306:308
Comment:
2-second hardcoded delay may be insufficient for high-latency networks or cause slow tests. Consider making this configurable via environment variable or using a retry/polling strategy to check if events were sent.
How can I resolve this? If you propose a fix, please make it concise.|
@hpouillot Addressed your comments |
hpouillot
left a comment
There was a problem hiding this comment.
I'm not a big fan of requiring from dist 😬 but we can change that later
Problem
No automated way to verify this SDK conforms to the shared PostHog capture API contract across SDKs.
Changes
Integrate the SDK Test Harness to validate SDK compliance against the shared capture API contract.
This adds:
The test harness runs a mock PostHog server, exercises the SDK through the adapter, and verifies behavior matches the contract defined in CONTRACT.yaml (event format, retries, compression, deduplication, etc.).
Release info Sub-libraries affected
Libraries affected
Checklist