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); } /**