Save Your Future Self Some Debugging Time

In a previous article we implemented the following method:

1
2
3
4
5
6
7
8
9
10
11
12
13
def self.delivering_email(message)
  # Dynamic settings are currently only used in production, but we want to check for a
  # valid sender domain in all environments to make sure we catch missing senders in the
  # development and test, too.
  /@(?<sender_domain>.+)/ =~ message.sender
    or raise "No valid sender for determining dynamic SMTP settings: `#{message.sender}`"

  if Rails.configuration.respond_to?(:dynamic_smtp_settings)
    dynamic_settings = Rails.configuration.dynamic_smtp_settings[sender_domain]
      or raise "No dynamic smtp settings configured for `#{sender_domain}`"
    message.delivery_method.settings.merge!(dynamic_settings)
  end
end

Being an ActionMailer interceptor means this method is automatically (implicitly) called every time a mailer method has prepared a message, right before the mail is going to be sent. Our method then looks at the sender address and sets the SMTP credentials accordingly.

Why did we chose to have those two explicit safety checks in lines 6 and 10?

When writing code we always try to ask ourselves: “What are the most probable ‘errors’ in other parts of the application that could affect the code we’re currently writing?” (Note the quotation marks – we’re not talking about “real” errors here, but more about inadvertently ignoring assumptions that were made in other parts of the code.)

For the interceptor method we came up with two cases:

• Having a mailer method not setting the sender at all (unlike the from address, the sender address is purely optional).
• Having a typo in the sender domain (either when setting the sender in the mailer method, or when specifying the dynamic settings in the configuration file).

Especially the first case is very likely to happen sooner or later, because the interceptor method introduces a new hidden requirement into the application: From now on, every mailer method has to set the message’s sender.

In both error cases our method would fail with an exception. Before reading on: Can you determine the exact exception that would happen? (Hint: It’s not an immediately obvious one.)

Here’s the answer: Both cases would lead to an “TypeError: no implicit conversion of nil into Hash” exception in line 11:

• First case: If sender is not set, the regexp does not match, thus sender_domain ends up being nil.
• Second case: A typo either in the sender address or in the configuration file means that sender_domain is a string that does not match any key in the dynamic_smtp_settings hash.
• So in both cases dynamic_settings gets set to the result of accessing a non-existing key in the dynamic_smtp_settings hash (line 9). That means it would simply be nil.
• Thus in line 11 we effectively call .merge!(nil), which results in the aforementioned TypeError.

Imagine this happening at some point in the future were we may have long forgotten about the interceptor method: The error message “TypeError: no implicit conversion of nil into Hash” would not be helpful at all and it would take some time inspecting the stack trace to find out what really has happened.

In his great book Exceptional Ruby, Avdi Grimm brings up “five questions to ask yourself before writing code to raise an Exception”. The fifth question is:

#5: Would continuing result in a less informative exception?

Sometimes failing to raise an exception just results in things going wrong in less easy-to-diagnose ways down the road. In cases like this, it’s better to raise earlier than later.

This is exactly why we chose to implement those two checks and to give them nice expressive failure messages, too. Someday our future selves will be really happy about that.