Setting symfony_mailer_lite.settings.default_transport not guaranteed to be set

Created on 4 January 2024, 9 months ago
Updated 3 February 2024, 8 months ago

Problem/Motivation

The setting symfony_mailer_lite.settings.default_transport is not guaranteed to be set, and it not being set, or being set to an invalid value causes fatal error:

Error: Call to a member function getDsn() on null in Drupal\symfony_mailer_lite\Plugin\Mail\SymfonyMailer->mail() (line 254 of web/modules/contrib/symfony_mailer_lite/src/Plugin/Mail/SymfonyMailer.php).

Steps to reproduce

  • Install & Enable Drupal Symfony Mailer Lite
  • Head to admin/config/system/symfony-mailer-lite/transport
  • Add a new transport type
  • Delete the current default transport type
  • Attempt to send a test message

Mitigation

Choose one of the transports as default.

Proposed resolution

  1. Prevent the default transport from being deleted in UI.

Remaining tasks

All

User interface changes

n/a

API changes

n/a

Data model changes

n/a

πŸ› Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡¦πŸ‡ΊAustralia ELC

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ELC
  • πŸ‡ΊπŸ‡ΈUnited States zengenuity

    I would suggest we prevent the default transport from being deleted in the user interface. And then we also add a guard to prevent a fatal error if still somehow you have no default transport. I don't think we should automatically switch transports or use another transport if you haven't specifically chosen it, but we can put a more useful error in the logs and just fail to send the message without a fatal error.

  • Status changed to Needs review 9 months ago
  • πŸ‡¦πŸ‡ΊAustralia ELC

    Code regarding handling mail when default transport does not exist has already been changed enough to gracefully handle this happenstance.

    This MR goes towards preventing the admin from deleting the default transport.

  • πŸ‡¦πŸ‡ΊAustralia ELC

    Looks like this one missed out on making into the next release. Updated MR to latest head.

  • πŸ‡ΊπŸ‡ΈUnited States zengenuity

    Sorry, I didn't include this in the most recent release because I don't think the way you did it was the best approach. I didn't reply yet because I needed more time to confirm that a different way would work.

    The better way to do this is to add an access control handler to the Transport entity type. Then, we can specify the logic for when deletes are allowed, and it updates everywhere necessary, such as removing the links and access to the form.

    I updated the MR, reverting your commits and adding the access control handler. Can you test my changes to make sure they work for you?

    Then, I'll merge this in.

  • Status changed to RTBC 8 months ago
  • πŸ‡¦πŸ‡ΊAustralia ELC

    That is a way better way to handle it. I had a bell going off in my head that what I written was missing something as the entity could still be deleted via code, and access handling was being done in forms. I knew better.

    Small adjustment as the delete operation in the list builder is also taken care of by the new access handler.

    Anyway, this works well now.

  • Pipeline finished with Skipped
    8 months ago
    #80289
  • Status changed to Fixed 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States zengenuity

    Good catch on the operation link. I've merged the MR. Thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024