Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document TransportFactoryInterface: Adding Socket Timeout Option to Mailer #40

Closed
nd4c opened this issue Aug 15, 2022 · 5 comments
Closed
Labels
type:docs Documentation

Comments

@nd4c
Copy link

nd4c commented Aug 15, 2022

What steps will reproduce the problem?

Migrate from Swift Mailer to Symfony Mailer

What's expected?

SwiftMailer has a transport->timeout property that can be set in the config. Defaults to 30.

What do you get instead?

The Symfony Mailer does not have an exposed way to set the timeout for the EsmtpTransport. (There is a value and a setter Method, but there is no way to call it since the transport is private...)

Additional info

If the Symfony Transport timeout property is not set it will default to the php.ini setting, 'default_socket_timeout' which default to 60.

We can wrap the mailer->send() call, but it would be nice to be able to config the Mailer's timeout value.

We cannot add a new property to the transport itself, but we can add one to the yii Mailer class.

Possible solution:

class Mailer extends BaseMailer
 {
    /**
     * @var int transport_timeout defaults to 0 since this would only apply to smtp/smtps transports.
     */
    public $transport_timeout = 0;
    ...
    /**
     * {@inheritDoc}
     *
     * @throws TransportExceptionInterface If sending failed.
     */
    protected function sendMessage($message): bool
    {
        ...
        try {
            if (!empty($this->transport_timeout) && (int) $this->transport_timeout > 0) {
                $default_socket_timeout = ini_set('default_socket_timeout', (int) $this->transport_timeout);
            }
            $this->getSymfonyMailer()->send($message);
        } catch (\Exception $exception) {
            Yii::getLogger()->log($exception->getMessage(), \yii\log\Logger::LEVEL_ERROR, __METHOD__);
            // need to re-throw the exception so that caller can extract transport errors
            throw($exception);
        } finally {
            if (!empty($default_socket_timeout) && (int) $default_socket_timeout > 0) {
                ini_set('default_socket_timeout', (int) $default_socket_timeout);
            }
        }
        return true;
    }
}
Q A
Yii version 2.0.45
Yii SymfonyMailer version 2.0.3
SymfonyMailer version v6.0.11
PHP version 8.1.9
Operating system Linux Mint 20.3 with Kernel Linux 5.15.0-46-generic
@nd4c nd4c mentioned this issue Aug 15, 2022
@SamMousa
Copy link
Contributor

Copied from #27 (comment)

You'd use a wrapper class (composition instead of inheritance) since Symfony even goes so far as to make their classes final so you cannot extend them.

class SmtpTransportWithTimeoutFactory implements TransportFactoryInterface {
    public function __construct(private TransportFactoryInterface $factory, private float $timeout)
    {
    }

    public function create(Dsn $dsn): TransportInterface
    {
        $result = $this->factory->create($dsn);
        if ($result instanceof SmtpTransport) {
            $result->getStream()->setTimeout($this->timeout);
        }
        return $result;
    }

    public function supports(Dsn $dsn): bool {
        return $this->factory->supports($dsn);
    }
}

@nd4c
Copy link
Author

nd4c commented Aug 16, 2022

Thanks! This will be a big help. I hadn't realized that Symfony classes were final. I'd suggest even including this Factory with the Yii2 extension.

@SamMousa
Copy link
Contributor

I'd suggest even including this Factory with the Yii2 extension.

I don't like that. There is an infinite number of options that consumers of the library might want to configure. Just let them build their own adapters, that's the clean way to integrate third party code.

@nd4c
Copy link
Author

nd4c commented Aug 16, 2022

I was thinking that lack of control for the timeout of the Symfony smtp/smtps Transport was a major hurdle for migrating from Swift to Symfony.

It might be helpful in the Migrating From yiisoft/yii2-swiftmailer section (https://github.com/yiisoft/yii2-symfonymailer#migrating-from-yiisoftyii2-swiftmailer) to include the sample wrapper and how to wire it up in the config file.

@SamMousa
Copy link
Contributor

Feel free to make a PR for the docs!

@samdark samdark added the type:docs Documentation label Aug 17, 2022
@samdark samdark changed the title Adding Socket Timeout Option to Mailer Document TransportFactoryInterface: Adding Socket Timeout Option to Mailer Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs Documentation
Projects
None yet
Development

No branches or pull requests

3 participants