Skip to content

feat: add email adapter DSN parsing#125

Open
deepshekhardas wants to merge 1 commit into
utopia-php:mainfrom
deepshekhardas:pr-117
Open

feat: add email adapter DSN parsing#125
deepshekhardas wants to merge 1 commit into
utopia-php:mainfrom
deepshekhardas:pr-117

Conversation

@deepshekhardas

Copy link
Copy Markdown

Add DSN (Data Source Name) parsing support for email adapters, enabling connection configuration via DSN strings.

@greptile-apps

greptile-apps Bot commented Jun 7, 2026

Copy link
Copy Markdown

Greptile Summary

Adds DSN-based construction for SMTP, Resend, Sendgrid, and Mailgun email adapters.

  • Adds parsing and validation helpers for DSN credentials and query options.
  • Introduces a multi-adapter Messenger with ordered failover.
  • Documents both features and adds their unit-test coverage.

Confidence Score: 2/5

The PR is not safe to merge until malformed DSN coverage, integer option validation, throwable failover, and sparse adapter-array handling are corrected.

The current code still admits invalid SMTP numeric settings, bypasses failover for adapter errors outside Exception, dereferences index zero in accepted sparse arrays, and retains a malformed-DSN test whose expected exception does not match the reached validation path.

Files Needing Attention: src/Utopia/Messaging/Adapter/Email.php, src/Utopia/Messaging/Messenger.php, tests/Messaging/Adapter/Email/DsnTest.php

Important Files Changed

Filename Overview
src/Utopia/Messaging/Adapter/Email.php Adds DSN parsing and adapter factories, but the previously reported integer range-validation defect remains.
src/Utopia/Messaging/Messenger.php Adds ordered adapter failover, but previously reported throwable handling and adapter-array normalization defects remain.
tests/Messaging/Adapter/Email/DsnTest.php Adds DSN coverage, including the previously reported malformed-SMTP assertion that remains inconsistent with the implementation.
tests/Messaging/MessengerTest.php Covers successful and failed adapter selection, validation, and aggregate errors but does not resolve the outstanding failover and sparse-array cases.

Reviews (5): Last reviewed commit: "feat: add email adapter DSN parsing" | Re-trigger Greptile

Comment thread tests/Messaging/Adapter/Email/DsnTest.php
Comment on lines +101 to +109
try {
return $adapter->send($message);
} catch (\Exception $e) {
$errors[] = $adapter->getName()
.' (adapter '
.($index + 1)
.'): '
.$e->getMessage();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The catch block only catches \Exception, so PHP \Error subclasses (\TypeError, \ValueError, \Error, etc.) thrown by an adapter will propagate uncaught and bypass the failover loop entirely. Catching \Throwable instead ensures all adapter-level failures are handled and the next adapter is tried.

Suggested change
try {
return $adapter->send($message);
} catch (\Exception $e) {
$errors[] = $adapter->getName()
.' (adapter '
.($index + 1)
.'): '
.$e->getMessage();
}
try {
return $adapter->send($message);
} catch (\Throwable $e) {
$errors[] = $adapter->getName()
.' (adapter '
.($index + 1)
.'): '
.$e->getMessage();
}

Comment on lines +198 to +209
private static function parseIntOption(mixed $value, string $option): int
{
if (\is_int($value)) {
return $value;
}

if (! \is_string($value) || $value === '' || ! \ctype_digit($value)) {
throw new \InvalidArgumentException('Invalid "'.$option.'" option. Expected integer value.');
}

return (int) $value;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 parseIntOption skips range validation when the value is already a PHP int. parse_url returns $parts['port'] as a native integer, so a URL like smtp://host:0 or smtp://host:99999 bypasses the ctype_digit check entirely and passes an invalid port directly to the SMTP constructor. The same zero/overflow gap applies to query-string ports since ctype_digit("0") and ctype_digit("99999") both return true. Adding a positive-integer guard closes this for all call sites (port, timeout, timelimit).

Suggested change
private static function parseIntOption(mixed $value, string $option): int
{
if (\is_int($value)) {
return $value;
}
if (! \is_string($value) || $value === '' || ! \ctype_digit($value)) {
throw new \InvalidArgumentException('Invalid "'.$option.'" option. Expected integer value.');
}
return (int) $value;
}
private static function parseIntOption(mixed $value, string $option): int
{
if (\is_int($value)) {
if ($value < 0) {
throw new \InvalidArgumentException('Invalid "'.$option.'" option. Expected non-negative integer value.');
}
return $value;
}
if (! \is_string($value) || $value === '' || ! \ctype_digit($value)) {
throw new \InvalidArgumentException('Invalid "'.$option.'" option. Expected integer value.');
}
return (int) $value;
}

Comment on lines +31 to +33
$this->validateAdapters($adapters);

$this->adapters = $adapters;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Normalize adapter arrays

Messenger accepts any array of adapters, but later reads $adapters[0]. An associative or sparse array like ['primary' => $adapter] or [1 => $adapter] passes the validation loop and then validateAdapters() tries to call methods on a missing index. Normalizing the array before storing or validating it lets named or sparse adapter lists work consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant