diff --git a/src/Appwrite/Event/Notification.php b/src/Appwrite/Event/Notification.php index acb4c8b62d..b7a31755ba 100644 --- a/src/Appwrite/Event/Notification.php +++ b/src/Appwrite/Event/Notification.php @@ -272,6 +272,7 @@ class Notification extends Event $this->name = ''; $this->subject = ''; $this->body = ''; + $this->preview = ''; $this->variables = []; $this->bodyTemplate = ''; $this->attachment = []; diff --git a/src/Appwrite/Platform/Modules/Account/Http/Alerts/Track/Get.php b/src/Appwrite/Platform/Modules/Account/Http/Alerts/Track/Get.php index 7c5a48346c..acad23c7d4 100644 --- a/src/Appwrite/Platform/Modules/Account/Http/Alerts/Track/Get.php +++ b/src/Appwrite/Platform/Modules/Account/Http/Alerts/Track/Get.php @@ -71,7 +71,8 @@ class Get extends Action $decoded = $decoder->decode($jwt); if ( - isset($decoded['alertId'], $decoded['userId']) + isset($decoded['alertId'], $decoded['userId'], $decoded['purpose']) + && $decoded['purpose'] === 'alert_track' && $decoded['alertId'] === $alertId ) { $authorization->skip(function () use ($dbForPlatform, $alertId, $decoded) { diff --git a/src/Appwrite/Platform/Workers/Notifications.php b/src/Appwrite/Platform/Workers/Notifications.php index b7d02fc03f..d9ac17e592 100644 --- a/src/Appwrite/Platform/Workers/Notifications.php +++ b/src/Appwrite/Platform/Workers/Notifications.php @@ -247,20 +247,20 @@ class Notifications extends Action } $subject = \strip_tags($subjectTemplate->render()); - // Persist alert BEFORE adapter send so the alertId is available for - // the tracking pixel. Failure to persist still allows the email to - // go out unsignals (we degrade gracefully). - $alertId = null; - if ($messageId !== '') { - $alertId = $this->persistAlert($dbForPlatform, $messageId, $recipient, $payload); - } + // Pre-compute the alertId WITHOUT persisting so the tracking pixel + // URL is stable, but defer the database write until after a + // successful adapter send. Persisting first risks a dedup-by-attribute + // hit on retry if SMTP throws — the email would be permanently lost. + $deterministicAlertId = $messageId !== '' + ? self::buildAlertId($messageId, $recipient) + : null; - // C3 tracking pixel: only injectable when we have a userId AND a - // persisted alertId AND a signing key. + // Tracking pixel: only injectable when we have a userId AND a + // deterministic alertId AND a signing key. $userId = $recipient['userId'] ?? ''; $opensslKey = System::getEnv('_APP_OPENSSL_KEY_V1'); - if ($alertId !== null && $userId !== '' && !empty($opensslKey)) { - $body = $this->injectTrackingPixel($body, $alertId, $userId, $opensslKey); + if ($deterministicAlertId !== null && $userId !== '' && !empty($opensslKey)) { + $body = $this->injectTrackingPixel($body, $deterministicAlertId, $userId, $opensslKey); } /** @var EmailAdapter $adapter */ @@ -320,13 +320,21 @@ class Notifications extends Action try { $adapter->send($emailMessage); } catch (Throwable $error) { - if ($type === 'smtp') { - throw new Exception('Error sending notification: ' . $error->getMessage(), 401); - } + // Use 500 for any SMTP delivery failure: timeouts, DNS, refused + // connections, and authentication errors are all infrastructure + // problems, not HTTP-style 401s. The queue infrastructure handles + // retries based on the throw, not the code. throw new Exception('Error sending notification: ' . $error->getMessage(), 500); } - return $alertId; + // Persist the alert ONLY after a successful send. The Track endpoint + // tolerates a missing alert row, so a tracking pixel fetched between + // send and persistence is harmless. + if ($messageId !== '') { + return $this->persistAlert($dbForPlatform, $messageId, $recipient, $payload); + } + + return null; } /** @@ -448,10 +456,10 @@ class Notifications extends Action // userId is supplied (matches Console adapter's own bookkeeping). if ($channel === NOTIFICATION_TYPE_CONSOLE && $userId === '' && $teamId === '') { $userId = $address; + $recipient['userId'] = $userId; } - $idSuffix = \substr(\md5($channel . ':' . $address . ':' . $userId . ':' . $teamId), 0, 8); - $alertId = $messageId . '_' . $idSuffix; + $alertId = self::buildAlertId($messageId, $recipient); $permissions = $this->buildAlertPermissions($userId, $teamId); if (empty($permissions)) { @@ -481,6 +489,24 @@ class Notifications extends Action } } + /** + * Build the deterministic alertId composed of the messageId plus an + * 8-char hash over the recipient identity. Single source of truth used + * by both `persistAlert()` and `dispatchEmail()` so the tracking pixel + * URL matches the eventually-persisted row exactly. + * + * @param array{address: string, channel: string, signatureKey?: string, userId?: string, teamId?: string} $recipient + */ + private static function buildAlertId(string $messageId, array $recipient): string + { + $channel = $recipient['channel']; + $address = $recipient['address']; + $userId = $recipient['userId'] ?? ''; + $teamId = $recipient['teamId'] ?? ''; + + return $messageId . '_' . \substr(\md5($channel . ':' . $address . ':' . $userId . ':' . $teamId), 0, 8); + } + /** * @return array */ @@ -542,6 +568,7 @@ class Notifications extends Action ->encode([ 'alertId' => $alertId, 'userId' => $userId, + 'purpose' => 'alert_track', ]); $protocol = System::getEnv('_APP_OPTIONS_FORCE_HTTPS', 'disabled') === 'disabled' ? 'http' : 'https'; diff --git a/src/Appwrite/Platform/Workers/Webhooks.php b/src/Appwrite/Platform/Workers/Webhooks.php index c5abf4f755..91485c2906 100644 --- a/src/Appwrite/Platform/Workers/Webhooks.php +++ b/src/Appwrite/Platform/Workers/Webhooks.php @@ -209,6 +209,11 @@ class Webhooks extends Action */ public function sendAlert(int $attempts, mixed $statusCode, Document $webhook, Document $project, Database $dbForPlatform, Notification $queueForNotifications, array $plan): void { + // The DI-shared Notification event accumulates state across calls. Reset + // before configuring this alert so multiple webhook failures in a single + // worker invocation do not bleed recipients/subject/body between alerts. + $queueForNotifications->reset(); + $memberships = $dbForPlatform->find('memberships', [ Query::equal('teamInternalId', [$project->getAttribute('teamInternalId')]), Query::limit(APP_LIMIT_SUBQUERY) diff --git a/src/Appwrite/Utopia/Messaging/Adapter/Console.php b/src/Appwrite/Utopia/Messaging/Adapter/Console.php index 28765e3242..bb705e59e4 100644 --- a/src/Appwrite/Utopia/Messaging/Adapter/Console.php +++ b/src/Appwrite/Utopia/Messaging/Adapter/Console.php @@ -5,6 +5,7 @@ namespace Appwrite\Utopia\Messaging\Adapter; use Appwrite\Utopia\Messaging\Messages\Console as ConsoleMessage; use Utopia\Database\Database; use Utopia\Database\Document; +use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; @@ -84,6 +85,12 @@ class Console extends Adapter $this->database->createDocument('alerts', $document); $delivered++; $response->addResult($key); + } catch (DuplicateException) { + // Idempotent retry: row already exists for this messageId/recipient. + // Treat as a successful (already-delivered) result so the worker + // does not throw and re-queue. + $delivered++; + $response->addResult($key); } catch (\Throwable $error) { $response->addResult($key, $error->getMessage()); }