feat(reddit-conversions-api): add Conversions API v3 support behind feature flag - #3848
Open
harsh-joshi99 wants to merge 10 commits into
Open
feat(reddit-conversions-api): add Conversions API v3 support behind feature flag#3848harsh-joshi99 wants to merge 10 commits into
harsh-joshi99 wants to merge 10 commits into
Conversation
- Add REDDIT_CONVERSIONS_CANARY_API_VERSION (v3.0) to versioning-info.ts - Implement feature flag 'reddit-conversions-api-canary-version' via getApiVersion(features) helper in utils.ts - Thread features through send() and both actions' perform/performBatch (standardEvent, customEvent) - testAuthentication intentionally remains on stable v2.0 - Update existing tests to use API_VERSION constant - Add stable + canary feature-flag tests for both actions - All 23 tests passing Breaking changes analysis in breaking-changes-analysis.md (changelog requires manual review — Reddit API host not reachable from build env) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Upgrades the Reddit Conversions API request URL to support a canary v3.0 endpoint behind the reddit-conversions-api-canary-version feature flag, while keeping stable behavior on v2.0 by default.
Changes:
- Added stable and canary API version constants and a
getApiVersion(features)helper to select the version at runtime. - Threaded
featuresthroughstandardEvent/customEventactions into the sharedsend()helper. - Updated and extended unit tests to validate stable vs canary URL selection.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/reddit-conversions-api/versioning-info.ts | Adds a canary v3.0 version constant alongside stable v2.0. |
| packages/destination-actions/src/destinations/reddit-conversions-api/utils.ts | Introduces feature-flag-driven API version selection and threads features into send(). |
| packages/destination-actions/src/destinations/reddit-conversions-api/standardEvent/index.ts | Passes features into send() for single and batch sends. |
| packages/destination-actions/src/destinations/reddit-conversions-api/customEvent/index.ts | Passes features into send() for single and batch sends. |
| packages/destination-actions/src/destinations/reddit-conversions-api/breaking-changes-analysis.md | Adds a manual checklist/process doc for validating v3.0 breaking changes. |
| packages/destination-actions/src/destinations/reddit-conversions-api/tests/index.test.ts | Refactors URL assertions to use constants and adds feature-flag URL tests. |
Comment on lines
+7
to
+11
| /** REDDIT_CONVERSIONS_CANARY_API_VERSION | ||
| * Reddit conversions API version (canary/feature-flagged). | ||
| * Testing new version v3.0 behind feature flag. | ||
| * API reference: https://ads-api.reddit.com/docs/v2/changelog | ||
| */ |
Comment on lines
+931
to
+933
| describe('standardEvent', () => { | ||
| it('should use the stable API version by default', async () => { | ||
| nock('https://ads-api.reddit.com').post(`/api/${API_VERSION}/conversions/events/ad_account_id_1`).reply(200, {}) |
Comment on lines
+947
to
+950
| it('should use the canary API version when the feature flag is enabled', async () => { | ||
| nock('https://ads-api.reddit.com') | ||
| .post(`/api/${CANARY_API_VERSION}/conversions/events/ad_account_id_1`) | ||
| .reply(200, {}) |
Comment on lines
+966
to
+968
| describe('customEvent', () => { | ||
| it('should use the stable API version by default', async () => { | ||
| nock('https://ads-api.reddit.com').post(`/api/${API_VERSION}/conversions/events/ad_account_id_1`).reply(200, {}) |
Comment on lines
+982
to
+985
| it('should use the canary API version when the feature flag is enabled', async () => { | ||
| nock('https://ads-api.reddit.com') | ||
| .post(`/api/${CANARY_API_VERSION}/conversions/events/ad_account_id_1`) | ||
| .reply(200, {}) |
Comment on lines
+22
to
+26
| > **Action required before merge / promoting the canary to stable:** a maintainer must | ||
| > manually review the v3.0 changelog at https://ads-api.reddit.com/docs/v2/changelog | ||
| > (and any v3 migration guide Reddit publishes) and complete the breaking-changes | ||
| > checklist below. Do not enable the feature flag for production traffic until this | ||
| > review is done. |
…e changelog analysis - Path segment is 'v3' not 'v3.0' (changelog: /api/v2.0/ -> /api/v3/) - Complete breaking-changes-analysis.md from the v3 changelog: all Conversions API changes v2.0 -> v3 are additive; only the URL path version segment changes. Risk downgraded to LOW. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
harsh-joshi99
marked this pull request as draft
June 30, 2026 05:25
Adds stable + canary feature-flag tests for the performBatch path on both standardEvent and customEvent, exercising the features-driven version selection threaded into send() (addresses PR review feedback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Isolate v2 and v3 into separate modules with a single flag fork in utils.ts:
- shared.ts: helpers reused by both (hashing, clean, getAdId, field types)
- v2.ts: existing v2.0 payload + URL, moved verbatim (sendV2)
- v3.ts: v3 payload (sendV3) — /pixels/{id}/conversion_events, { data: { events } }
envelope, event_at as strict epoch-ms, event_type->type, event_metadata->metadata,
value_decimal->value, UPPER_SNAKE tracking_type, action_source, event_source_url, test_id
- utils.send() forks on the canary flag; actions unchanged
New optional/defaulted fields (additive, only sent on v3): action_source (required,
default WEBSITE), event_source_url, test_id, product quantity/item_price.
Error handling relies on framework defaults (4xx no-retry except 408/423/429, 5xx retry),
which already match Reddit's v3 error semantics.
Tests: v2 regression suite unchanged; canary URL fixed to the v3 path; added v3
payload-shape tests (envelope, epoch-ms, UPPER_SNAKE, custom->CUSTOM, test_id). 30 passing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+24
to
+25
| const V3_URL = (pixelId: string) => | ||
| `https://ads-api.reddit.com/api/${REDDIT_CONVERSIONS_CANARY_API_VERSION}/pixels/${pixelId}/conversion_events` |
Comment on lines
+40
to
+42
| export async function sendV3(request: RequestClient, settings: Settings, payload: StandardEvent[] | CustomEvent[]) { | ||
| const data = createRedditPayloadV3(payload) | ||
| return request(V3_URL(settings.ad_account_id), { |
| // v3 requires event_at as an integer Unix epoch in milliseconds. We own the | ||
| // timestamp source (defaults to $.timestamp, an ISO string), so we accept ISO | ||
| // strings and 13-digit epoch-ms; anything else is rejected rather than sent wrong. | ||
| export function toEpochMs(value: string | number | undefined): number { |
Comment on lines
+94
to
+95
| if (typeof value === 'number' && Number.isInteger(value)) return value | ||
| if (typeof value === 'string' && /^\d{13}$/.test(value.trim())) return Number(value.trim()) |
| // v3 requires event_at as an integer Unix epoch in milliseconds. We own the | ||
| // timestamp source (defaults to $.timestamp, an ISO string), so we accept ISO | ||
| // strings and 13-digit epoch-ms; anything else is rejected rather than sent wrong. | ||
| export function toEpochMs(value: string | number | undefined): number { |
Comment on lines
+101
to
+104
| throw new PayloadValidationError( | ||
| `event_at must be an ISO 8601 timestamp or epoch milliseconds, received: ${String(value)}` | ||
| ) | ||
| } |
…aking existing actions action_source is required by Reddit v3 but adding a required field to the existing standardEvent/customEvent actions is a breaking change (per CLAUDE.md) and would surface as a required-but-unused field for all v2 users. Make it optional with a WEBSITE default; enforce presence at runtime in the v3 payload builder instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const custom_event_name = clean((payload as CustomEvent).custom_event_name) | ||
| const tracking_type = custom_event_name ? 'Custom' : (payload as StandardEvent).tracking_type | ||
|
|
||
| if (!action_source) throw new PayloadValidationError('action_source is required') |
Comment on lines
+24
to
+25
| const V3_URL = (pixelId: string) => | ||
| `https://ads-api.reddit.com/api/${REDDIT_CONVERSIONS_CANARY_API_VERSION}/pixels/${pixelId}/conversion_events` |
|
|
||
| export async function sendV3(request: RequestClient, settings: Settings, payload: StandardEvent[] | CustomEvent[]) { | ||
| const data = createRedditPayloadV3(payload) | ||
| return request(V3_URL(settings.ad_account_id), { |
Contributor
|
Manik [Reddit] here, |
Contributor
|
@harsh-joshi99 am I allowed to make changes directly to this branch? |
… comments - Delete breaking-changes-analysis.md: written under the initial (wrong) assumption that v3 was only a URL version swap; v3 is a full payload rewrite, so the doc is inaccurate and misleading. - index.ts: drop dangling reference to a local-only doc in the test_mode comment. - versioning-info.ts: correct the canary comment to describe the real v3 endpoint/payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
harsh-joshi99
changed the title
Comment on lines
+9
to
+11
| * v3 is a breaking payload rewrite (new endpoint /api/v3/pixels/{pixel_id}/conversion_events, | ||
| * `data` envelope, renamed/retyped fields). See v3.ts for the transform. | ||
| * API reference: https://ads-api.reddit.com/docs/v3/capi-migration |
Comment on lines
+24
to
+25
| const V3_URL = (pixelId: string) => | ||
| `https://ads-api.reddit.com/api/${REDDIT_CONVERSIONS_CANARY_API_VERSION}/pixels/${pixelId}/conversion_events` |
|
|
||
| export async function sendV3(request: RequestClient, settings: Settings, payload: StandardEvent[] | CustomEvent[]) { | ||
| const data = createRedditPayloadV3(payload) | ||
| return request(V3_URL(settings.ad_account_id), { |
| const custom_event_name = clean((payload as CustomEvent).custom_event_name) | ||
| const tracking_type = custom_event_name ? 'Custom' : (payload as StandardEvent).tracking_type | ||
|
|
||
| if (!action_source) throw new PayloadValidationError('action_source is required') |
| } | ||
|
|
||
| function createRedditPayloadV3(payloads: StandardEvent[] | CustomEvent[]): V3Payload { | ||
| const test_id = clean(payloads[0]?.test_id) |
| } | ||
| }) | ||
|
|
||
| return { data: { events, partner: 'SEGMENT', test_id } } |
| if (!dataProcessingOptions) return undefined | ||
| return { | ||
| country: clean(dataProcessingOptions.country), | ||
| modes: dataProcessingOptions.modes?.split(',').map((mode) => mode.trim()), |
| User, | ||
| Product, | ||
| EventMetadata, | ||
| DatapProcessingOptions |
|
|
||
| function getDataProcessingOptions( | ||
| dataProcessingOptions: DataProcessingOptionsType | ||
| ): DatapProcessingOptions | undefined { |
Comment on lines
+91
to
+97
| export function toEpochMs(value: string | number | undefined): number { | ||
| if (value === undefined || value === null || value === '') { | ||
| throw new PayloadValidationError('event_at is required') | ||
| } | ||
| // Already epoch milliseconds (number or 13-digit numeric string). | ||
| if (typeof value === 'number' && Number.isInteger(value)) return value | ||
| if (typeof value === 'string' && /^\d{13}$/.test(value.trim())) return Number(value.trim()) |
Comment on lines
+49
to
+50
| function createRedditPayloadV3(payloads: StandardEvent[] | CustomEvent[]): V3Payload { | ||
| const test_id = clean(payloads[0]?.test_id) |
Comment on lines
+141
to
+150
| function getDataProcessingOptions( | ||
| dataProcessingOptions: DataProcessingOptionsType | ||
| ): V3DataProcessingOptions | undefined { | ||
| if (!dataProcessingOptions) return undefined | ||
| return { | ||
| country: clean(dataProcessingOptions.country), | ||
| modes: dataProcessingOptions.modes?.split(',').map((mode) => mode.trim()), | ||
| region: clean(dataProcessingOptions.region) | ||
| } | ||
| } |
Comment on lines
+9
to
+10
| * v3 is a breaking payload rewrite (new endpoint /api/v3/pixels/{pixel_id}/conversion_events, | ||
| * `data` envelope, renamed/retyped fields). See v3.ts for the transform. |
|
|
||
| export const API_VERSION = REDDIT_CONVERSIONS_API_VERSION | ||
| export const CANARY_API_VERSION = REDDIT_CONVERSIONS_CANARY_API_VERSION | ||
| export const FLAGON_NAME = 'reddit-conversions-api-canary-version' |
Contributor
Author
Hey @ManikMM, sorry I missed your comment. I'm not entirely sure if your current permissions allow you to push directly here (cc: @joe-ayoub-segment), but feel free to cut a branch from this one. Let's connect over the email thread to schedule the meeting. |
Comment on lines
+91
to
+106
| export function toEpochMs(value: string | number | undefined): number { | ||
| if (value === undefined || value === null || value === '') { | ||
| throw new PayloadValidationError('event_at is required') | ||
| } | ||
| // Already epoch milliseconds (number or 13-digit numeric string). | ||
| if (typeof value === 'number' && Number.isInteger(value)) return value | ||
| if (typeof value === 'string' && /^\d{13}$/.test(value.trim())) return Number(value.trim()) | ||
| // ISO 8601 / RFC3339 string. | ||
| if (typeof value === 'string') { | ||
| const ms = Date.parse(value) | ||
| if (!Number.isNaN(ms)) return ms | ||
| } | ||
| throw new PayloadValidationError( | ||
| `event_at must be an ISO 8601 timestamp or epoch milliseconds, received: ${String(value)}` | ||
| ) | ||
| } |
Comment on lines
+49
to
+50
| function createRedditPayloadV3(payloads: StandardEvent[] | CustomEvent[]): V3Payload { | ||
| const test_id = clean(payloads[0]?.test_id) |
Comment on lines
+141
to
+150
| function getDataProcessingOptions( | ||
| dataProcessingOptions: DataProcessingOptionsType | ||
| ): V3DataProcessingOptions | undefined { | ||
| if (!dataProcessingOptions) return undefined | ||
| return { | ||
| country: clean(dataProcessingOptions.country), | ||
| modes: dataProcessingOptions.modes?.split(',').map((mode) => mode.trim()), | ||
| region: clean(dataProcessingOptions.region) | ||
| } | ||
| } |
Comment on lines
+5
to
+12
| import { | ||
| StandardEventPayloadItem, | ||
| StandardEventPayload, | ||
| User, | ||
| Product, | ||
| EventMetadata, | ||
| DatapProcessingOptions | ||
| } from './types' |
Comment on lines
+40
to
+47
| export async function sendV3(request: RequestClient, settings: Settings, payload: StandardEvent[] | CustomEvent[]) { | ||
| const data = createRedditPayloadV3(payload) | ||
| return request(V3_URL(settings.ad_account_id), { | ||
| method: 'POST', | ||
| headers: { Authorization: `Bearer ${settings.conversion_token}` }, | ||
| json: JSON.parse(JSON.stringify(data)) | ||
| }) | ||
| } |
…cion toEpochMs accepted any integer as epoch-ms, so a 10-digit epoch-seconds value would be sent as ms (timestamp in 1970). Require numeric input to be plausibly ms (>= 1e12) else throw PayloadValidationError. Add unit tests for the rejection paths (seconds, non-integer, unparseable, missing) plus the accepted cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+49
to
+52
| function createRedditPayloadV3(payloads: StandardEvent[] | CustomEvent[]): V3Payload { | ||
| const test_id = clean(payloads[0]?.test_id) | ||
|
|
||
| const events: V3EventItem[] = payloads.map((payload) => { |
Comment on lines
+85
to
+86
| return { data: { events, partner: 'SEGMENT', test_id } } | ||
| } |
Comment on lines
+61
to
+64
| conversion_id, | ||
| action_source, | ||
| event_source_url | ||
| } = payload |
Comment on lines
+69
to
+74
| if (!action_source) throw new PayloadValidationError('action_source is required') | ||
|
|
||
| return { | ||
| event_at: toEpochMs(event_at), | ||
| action_source, | ||
| event_source_url: clean(event_source_url), |
Comment on lines
+24
to
+25
| const V3_URL = (pixelId: string) => | ||
| `https://ads-api.reddit.com/api/${REDDIT_CONVERSIONS_CANARY_API_VERSION}/pixels/${pixelId}/conversion_events` |
Comment on lines
+40
to
+42
| export async function sendV3(request: RequestClient, settings: Settings, payload: StandardEvent[] | CustomEvent[]) { | ||
| const data = createRedditPayloadV3(payload) | ||
| return request(V3_URL(settings.ad_account_id), { |
Comment on lines
+99
to
+103
| export interface V3EventItem { | ||
| event_at: number | ||
| action_source: string | ||
| event_source_url?: string | ||
| click_id?: string |
harsh-joshi99
marked this pull request as ready for review
July 21, 2026 09:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for Reddit Conversions API v3 behind the feature flag
reddit-conversions-api-canary-version. With the flag off (default), the destination behaves exactly as today (v2.0). With the flag on, events are sent to the v3 endpoint with the v3 payload.What changed in v3 (vs v2.0)
/api/v2.0/conversions/events/{pixel}/api/v3/pixels/{pixel}/conversion_events{ events, test_mode, partner }{ data: { events, partner, test_id } }event_atevent_typetypetracking_typePageVisit…PAGE_VISIT…),Custom→CUSTOMevent_metadatametadatavalue_decimalvalueaction_sourceWEBSITE)test_mode(bool)test_idstringevent_source_url, productquantity/item_priceApproach: flag-gated, isolated modules
utils.ts— a single flag fork insend()→sendV3orsendV2.v2.ts— existing v2.0 logic, moved verbatim (unchanged behavior).v3.ts— new v3 payload builder + endpoint.shared.ts— helpers reused by both (hashing, cleaners, field types).This keeps v2 untouched and makes eventual cleanup a deletion (remove
v2.ts, collapse the fork) rather than untangling inline branches.New fields (additive, optional — safe for existing v2 mappings)
action_source— optional, defaults toWEBSITE(Reddit requires it on v3; enforced at runtime inv3.ts). Kept optional to avoid adding a required field to existing actions.test_id— optional; routes to Reddit Event Testing. Only sent on v3.event_source_url, productquantity/item_price— optional.All new fields are only sent on the v3 path; on v2 they're ignored.
Testing
event_at, UPPER_SNAKE,type/metadatarenames, custom→CUSTOM,test_id), both actions,perform+performBatch.200 Successfully processed✅POST /api/v3/pixels/{pixel}/conversion_events, correct v3 body,200 Successfully processed✅ (includingpartnerinsidedataandtest_idaccepted)Testing Document
Error handling
Relies on framework defaults, which match Reddit's v3 error semantics: 4xx not retried (except 408/423/429), 5xx retried. Reddit's documented codes (400 validation, 401/403 auth/scope, 429 rate limit, 500 downtime) map cleanly onto these.
Rollout / cleanup
v2.ts, collapse thesend()fork, remove the flag +test_modesetting.Open questions for Reddit (non-blocking, confirming)
partner: "SEGMENT"placement insidedata— accepted in staging (200); confirming it's the intended location.action_sourceoffline enum value — usingPHYSICAL_STORE; confirming exact wire value.🤖 Generated with Claude Code