From aba312c484948ed62f1f6e0300e12a1403146b15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 21 Sep 2026 11:13:21 +0200 Subject: [PATCH] Skip pixels without an access token instead of rejecting the whole event Since #17 a single pixel without an access token made the client throw before sending anything, so that one pixel kept every other pixel from receiving the event. A pixel without an access token is a legitimate state, a pixel that is only used in the browser for instance, so a shop with one such pixel next to fully configured ones got no server events at all. The client now sends to every pixel that has an access token, skips the ones that do not, and logs an error naming them. It throws an InvalidArgumentException only when none of the pixels has an access token, which is what happens when PreparedEvent::withAccessTokens() is forgotten after withoutAccessTokens(): then nothing can be delivered and it is almost certainly a mistake. A request without an access token is still never made, since Meta answers it with an error that does not mention the token. --- CLAUDE.md | 4 +- README.md | 10 ++-- UPGRADE-2.0.md | 14 ++++-- src/Client/Client.php | 21 ++++++-- src/Client/ClientInterface.php | 4 +- src/Exception/InvalidArgumentException.php | 2 +- src/Pixel/Pixel.php | 2 +- tests/Client/ClientTest.php | 57 ++++++++++++++++------ 8 files changed, 79 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 22795c3..f19c5af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,9 +54,9 @@ So to add a field: add the public property, map it in `getMapping()`, and regist **`Parameters` subclasses:** `Event` (the aggregate root — holds `User $userData`, `Custom $customData`, a list of `Pixel`, plus `metadata` for app-internal use that is never sent), `User` (customer matching data), `Custom` (event-specific data like value/currency/contents), `Content` (a single item in `Custom::$contents`). `Event` auto-generates `eventId` (random, for [deduplication](https://developers.facebook.com/docs/marketing-api/conversions-api/parameters/server-event#event-id)) and `eventTime` in its constructor. `Event` is intentionally **not** `final` so consumers can subclass it into domain-specific events; the other data objects are `final`. -**`Client` (`src/Client/Client.php`)** — `sendEvent()` delegates to `sendPreparedEvent($event->prepare())`. `Event::prepare()` returns a `PreparedEvent` (`src/Event/PreparedEvent.php`): the `getPayload()` array plus the delivery information (event name and id, pixels, test event code), made of scalars/arrays/`Pixel` only so consumers can hash at capture time and queue it. The pixels are cloned so it is a snapshot, and `withoutAccessTokens()`/`withAccessTokens()` (immutable) keep the tokens out of the queue and restore them by pixel id before sending. `sendPreparedEvent()` first rejects pixels without an access token with an `InvalidArgumentException`, before any request, so an event is never delivered to only some of its pixels (Meta's own error for a missing token does not mention the token). It then POSTs the payload (form-encoded) to `graph.facebook.com/v{ApiConfig::APIVersion}/{pixelId}/events` once per pixel (each pixel carries its own access token). A failure of the HTTP client is wrapped in a `TransportException`, and a non-200 response throws a `ResponseException` carrying the status code, the raw body and, when the body is in Meta's error format, the parsed `ErrorResponse`. HTTP is fully PSR-based: PSR-18 client and PSR-17 factories are auto-discovered via `php-http/discovery` but can be injected with `setHttpClient()` / `setRequestFactory()` / etc. The client is `LoggerAware` and defaults to `NullLogger`. +**`Client` (`src/Client/Client.php`)** — `sendEvent()` delegates to `sendPreparedEvent($event->prepare())`. `Event::prepare()` returns a `PreparedEvent` (`src/Event/PreparedEvent.php`): the `getPayload()` array plus the delivery information (event name and id, pixels, test event code), made of scalars/arrays/`Pixel` only so consumers can hash at capture time and queue it. The pixels are cloned so it is a snapshot, and `withoutAccessTokens()`/`withAccessTokens()` (immutable) keep the tokens out of the queue and restore them by pixel id before sending. `sendPreparedEvent()` skips pixels without an access token and logs an error naming them (Meta's own error for a missing token does not mention the token, and a token-less pixel is legitimate, e.g. browser-only, so it must not block the others); only when no pixel has a token does it throw an `InvalidArgumentException`, before any request. It then POSTs the payload (form-encoded) to `graph.facebook.com/v{ApiConfig::APIVersion}/{pixelId}/events` once per pixel (each pixel carries its own access token). A failure of the HTTP client is wrapped in a `TransportException`, and a non-200 response throws a `ResponseException` carrying the status code, the raw body and, when the body is in Meta's error format, the parsed `ErrorResponse`. HTTP is fully PSR-based: PSR-18 client and PSR-17 factories are auto-discovered via `php-http/discovery` but can be injected with `setHttpClient()` / `setRequestFactory()` / etc. The client is `LoggerAware` and defaults to `NullLogger`. -**Exceptions (`src/Exception/`)** — everything the SDK throws implements `ExceptionInterface`. There are three concrete classes, one per thing a caller can do: `InvalidArgumentException` (extends SPL's; the caller's fault, never retry: bad cookie values, invalid event data, pixels without an access token, an unencodable payload), `TransportException` (no response; retry) and `ResponseException` (non-200; decide from `statusCode`/`errorResponse`). To keep that promise, never throw SPL exceptions or use `Webmozart\Assert\Assert` directly in `src/`: use `Setono\MetaConversionsApi\Assert`, an internal subclass whose failures throw the SDK's `InvalidArgumentException`, and wrap third-party exceptions (the Facebook `Normalizer`, PSR-18, `\JsonException`). `FbqGenerator` is the one place that does not throw: its output goes straight into a page, so it logs and returns an empty string when the data cannot be encoded. +**Exceptions (`src/Exception/`)** — everything the SDK throws implements `ExceptionInterface`. There are three concrete classes, one per thing a caller can do: `InvalidArgumentException` (extends SPL's; the caller's fault, never retry: bad cookie values, invalid event data, an event none of whose pixels has an access token, an unencodable payload), `TransportException` (no response; retry) and `ResponseException` (non-200; decide from `statusCode`/`errorResponse`). To keep that promise, never throw SPL exceptions or use `Webmozart\Assert\Assert` directly in `src/`: use `Setono\MetaConversionsApi\Assert`, an internal subclass whose failures throw the SDK's `InvalidArgumentException`, and wrap third-party exceptions (the Facebook `Normalizer`, PSR-18, `\JsonException`). `FbqGenerator` is the one place that does not throw: its output goes straight into a page, so it logs and returns an empty string when the data cannot be encoded. **`FbqGenerator` (`src/Generator/FbqGenerator.php`)** — the client-side counterpart. Generates the `fbq('init', ...)` / `fbq('track', ...)` JavaScript snippets, using the browser-context payload and reusing the same `eventId` so server and browser events deduplicate. `Event::isCustom()` decides between `track` and `trackCustom`. diff --git a/README.md b/README.md index 4decaed..3d810c0 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ one place. There are three concrete exceptions, one for each thing you can do ab | Exception | Thrown when | What to do | |---|---|---| -| `InvalidArgumentException` | The SDK is given something it cannot work with: a pixel without an access token, event data Meta does not accept, a payload that cannot be encoded, a cookie value in the wrong format. Always thrown before any request is made | Fix the input. Retrying will not help | +| `InvalidArgumentException` | The SDK is given something it cannot work with: an event none of whose pixels has an access token, event data Meta does not accept, a payload that cannot be encoded, a cookie value in the wrong format. Always thrown before any request is made | Fix the input. Retrying will not help | | `TransportException` | The request never got a response, e.g. a network error or a timeout. The exception from your HTTP client is the previous exception | Retry | | `ResponseException` | Meta, or a proxy in between, answered with anything but a 200 | Decide from `$e->statusCode` and `$e->errorResponse` | @@ -187,8 +187,10 @@ $client->sendPreparedEvent($preparedEvent->withAccessTokens([ `withAccessTokens()` takes the tokens indexed by pixel id and leaves pixels that are not in the list as they are. Both return a new instance. If your queue is trusted with the access tokens, you can skip both calls. -If a pixel still has no access token when you send, the client throws an `InvalidArgumentException` that names the -pixel, before any request is made. The event is therefore never delivered to only some of its pixels. +Pixels that still have no access token when you send are skipped, and the client logs an error that names them. A +pixel that is only used in the browser therefore does not keep the other pixels from receiving the event. If none of +the pixels has an access token, which is what happens when `withAccessTokens()` is forgotten, the client throws an +`InvalidArgumentException` before any request is made. ## Browser-side tracking with deduplication @@ -257,7 +259,7 @@ $client->setStreamFactory($myPsr17StreamFactory); ## Logging `Client` is `LoggerAware`. Pass any PSR-3 logger and the SDK will, for example, warn you when you try to send an event -that has no pixels associated: +that has no pixels associated, or when it skips a pixel because it has no access token: ```php $client->setLogger($logger); diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md index c41fd9e..f684e18 100644 --- a/UPGRADE-2.0.md +++ b/UPGRADE-2.0.md @@ -45,13 +45,17 @@ What to change: When the custom data cannot be encoded as JSON, it now logs an error and returns an empty string, which is what `generateInit()` already did. The output of both goes straight into a page, where an exception would break the page. -## Pixels without an access token are rejected before any request is made +## Pixels without an access token are skipped In 1.x the client sent the request anyway, and Meta answered with an error that does not mention the access token -("Unsupported post request. Object with ID ... does not exist, cannot be loaded due to missing permissions ..."). In 2.0 -the client throws an `InvalidArgumentException` naming the pixels instead. With several pixels there is one more -difference: all pixels are checked first, so the pixels listed before the one without an access token no longer receive -the event. +("Unsupported post request. Object with ID ... does not exist, cannot be loaded due to missing permissions ..."). That +exception also kept the pixels listed after it from receiving the event. + +In 2.0 the client sends to every pixel that has an access token, skips the ones that do not, and logs an error naming +them. A pixel without an access token is a legitimate state, for a pixel that is only used in the browser for instance, +so it no longer gets in the way of the others. Only when none of the pixels has an access token does the client throw +an `InvalidArgumentException`, before any request is made. That is what you will see if you forget +`PreparedEvent::withAccessTokens()` after `withoutAccessTokens()`. ## Behaviour change in `Client::sendEvent()` diff --git a/src/Client/Client.php b/src/Client/Client.php index 78f6f8c..95ce478 100644 --- a/src/Client/Client.php +++ b/src/Client/Client.php @@ -48,18 +48,29 @@ public function sendPreparedEvent(PreparedEvent $preparedEvent): void return; } - // Meta rejects a request without an access token, and with an error that does not mention the token. - // All pixels are checked before anything is sent, so that the event is never delivered to only some of them + // Meta rejects a request without an access token, and with an error that does not mention the token, so such + // a pixel is never sent to. It is a legitimate state though, e.g. for a pixel that is only used in the browser, + // and it must not keep the other pixels from receiving the event + $pixels = []; $pixelIdsWithoutAccessToken = []; foreach ($preparedEvent->pixels as $pixel) { if (null === $pixel->accessToken || '' === $pixel->accessToken) { $pixelIdsWithoutAccessToken[] = $pixel->id; + } else { + $pixels[] = $pixel; } } - if ([] !== $pixelIdsWithoutAccessToken) { + if ([] === $pixels) { throw new InvalidArgumentException(sprintf( - 'The event was not sent to Meta/Facebook because these pixels have no access token: %s. If the access tokens were removed with PreparedEvent::withoutAccessTokens(), add them back with PreparedEvent::withAccessTokens() before sending', + 'The event was not sent to Meta/Facebook because none of its pixels has an access token: %s. If the access tokens were removed with PreparedEvent::withoutAccessTokens(), add them back with PreparedEvent::withAccessTokens() before sending', + implode(', ', $pixelIdsWithoutAccessToken), + )); + } + + if ([] !== $pixelIdsWithoutAccessToken) { + $this->logger->error(sprintf( + 'The event was not sent to these pixels because they have no access token: %s', implode(', ', $pixelIdsWithoutAccessToken), )); } @@ -76,7 +87,7 @@ public function sendPreparedEvent(PreparedEvent $preparedEvent): void ), previous: $e); } - foreach ($preparedEvent->pixels as $pixel) { + foreach ($pixels as $pixel) { $body = [ 'access_token' => $pixel->accessToken, 'data' => $data, diff --git a/src/Client/ClientInterface.php b/src/Client/ClientInterface.php index a9305b5..0ed6094 100644 --- a/src/Client/ClientInterface.php +++ b/src/Client/ClientInterface.php @@ -14,7 +14,7 @@ interface ClientInterface { /** - * @throws ExceptionInterface if the event's data is invalid, a pixel has no access token, or the request failed in any way + * @throws ExceptionInterface if the event's data is invalid, none of the pixels has an access token, or the request failed in any way */ public function sendEvent(Event $event): void; @@ -22,7 +22,7 @@ public function sendEvent(Event $event): void; * Sends an event that was prepared earlier with Event::prepare(). Use this when the personal data is hashed * at capture time and the event is sent later, for instance through a queue * - * @throws ExceptionInterface if a pixel has no access token, the payload cannot be encoded, or the request failed in any way + * @throws ExceptionInterface if none of the pixels has an access token, the payload cannot be encoded, or the request failed in any way */ public function sendPreparedEvent(PreparedEvent $preparedEvent): void; } diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index 9826fcb..cc87d02 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -6,7 +6,7 @@ /** * Thrown when the SDK is given something it cannot work with: a cookie value in the wrong format, event data Meta - * does not accept, a pixel without an access token, and so on. The caller has to fix the input; retrying will not help + * does not accept, an event none of whose pixels has an access token, and so on. The caller has to fix the input; retrying will not help */ final class InvalidArgumentException extends \InvalidArgumentException implements ExceptionInterface { diff --git a/src/Pixel/Pixel.php b/src/Pixel/Pixel.php index 4726a24..f373c05 100644 --- a/src/Pixel/Pixel.php +++ b/src/Pixel/Pixel.php @@ -16,7 +16,7 @@ final class Pixel * but first available when you want to send the event, hence you populate it later * 3. There's the risk of having the access token being outputted in some user facing error message * - * The client refuses to send an event to a pixel without an access token + * The client skips a pixel without an access token when it sends an event */ public ?string $accessToken; diff --git a/tests/Client/ClientTest.php b/tests/Client/ClientTest.php index 57f54c0..3848cd7 100644 --- a/tests/Client/ClientTest.php +++ b/tests/Client/ClientTest.php @@ -227,7 +227,7 @@ public function it_sends_a_prepared_event_that_was_queued_without_its_access_tok /** * @test */ - public function it_throws_when_a_pixel_has_no_access_token(): void + public function it_throws_when_none_of_the_pixels_has_an_access_token(): void { $httpClient = new TestHttpClient(); @@ -235,13 +235,15 @@ public function it_throws_when_a_pixel_has_no_access_token(): void $client->setHttpClient($httpClient); $event = new Event(Event::EVENT_PURCHASE); - $event->pixels[] = new Pixel('pixel_id'); + $event->pixels[] = new Pixel('pixel_1', 'token_1'); + $event->pixels[] = new Pixel('pixel_2', 'token_2'); try { - $client->sendEvent($event); + // withAccessTokens() was forgotten + $client->sendPreparedEvent($event->prepare()->withoutAccessTokens()); self::fail('Expected an InvalidArgumentException'); } catch (InvalidArgumentException $e) { - self::assertStringContainsString('these pixels have no access token: pixel_id.', $e->getMessage()); + self::assertStringContainsString('none of its pixels has an access token: pixel_1, pixel_2.', $e->getMessage()); } self::assertCount(0, $httpClient->requests); @@ -250,30 +252,55 @@ public function it_throws_when_a_pixel_has_no_access_token(): void /** * @test */ - public function it_sends_nothing_when_any_of_the_pixels_has_no_access_token(): void + public function it_skips_the_pixels_without_an_access_token_and_sends_to_the_others(): void { $httpClient = new TestHttpClient(); + $logger = new TestLogger(); $client = new Client(); $client->setHttpClient($httpClient); + $client->setLogger($logger); + + // the constructor turns an empty string into null, but the property is public + $pixelWithAnEmptyAccessToken = new Pixel('pixel_4', 'token_4'); + $pixelWithAnEmptyAccessToken->accessToken = ''; $preparedEvent = new PreparedEvent( Event::EVENT_PURCHASE, 'event_id', ['event_name' => 'Purchase'], - [new Pixel('pixel_1', 'token_1'), new Pixel('pixel_2'), new Pixel('pixel_3', 'token_3'), new Pixel('pixel_4')], + // pixel_2 is only used in the browser + [new Pixel('pixel_1', 'token_1'), new Pixel('pixel_2'), new Pixel('pixel_3', 'token_3'), $pixelWithAnEmptyAccessToken], ); - try { - // pixel_2 and pixel_4 were not in the list, so they are still without an access token - $client->sendPreparedEvent($preparedEvent->withoutAccessTokens()->withAccessTokens(['pixel_1' => 'token_1', 'pixel_3' => 'token_3'])); - self::fail('Expected an InvalidArgumentException'); - } catch (InvalidArgumentException $e) { - self::assertStringContainsString('these pixels have no access token: pixel_2, pixel_4.', $e->getMessage()); - } + $client->sendPreparedEvent($preparedEvent); - // not even pixel_1, which comes first and has an access token, received the event - self::assertCount(0, $httpClient->requests); + self::assertCount(2, $httpClient->requests); + [$first, $second] = $httpClient->requests; + self::assertSame(sprintf('https://graph.facebook.com/v%s/pixel_1/events', ApiConfig::APIVersion), (string) $first->getUri()); + self::assertStringContainsString('access_token=token_1', (string) $first->getBody()); + self::assertSame(sprintf('https://graph.facebook.com/v%s/pixel_3/events', ApiConfig::APIVersion), (string) $second->getUri()); + self::assertStringContainsString('access_token=token_3', (string) $second->getBody()); + + self::assertSame(['The event was not sent to these pixels because they have no access token: pixel_2, pixel_4'], $logger->messages); + } + + /** + * @test + */ + public function it_logs_nothing_when_all_pixels_have_an_access_token(): void + { + $logger = new TestLogger(); + + $client = new Client(); + $client->setHttpClient(new TestHttpClient()); + $client->setLogger($logger); + + $event = new Event(Event::EVENT_PURCHASE); + $event->pixels[] = new Pixel('pixel_id', 'access_token'); + $client->sendEvent($event); + + self::assertSame([], $logger->messages); } /**