Skip to content

Commit

Permalink
Make exception API compatible with what Ruby expects
Browse files Browse the repository at this point in the history
Ruby expects that the first argument to Exception#initialize is a
string (or nil), since that is what Kernel#raise uses.  Breaking that
assumption is a very bad idea.

This problem was introduced in 16be09a60c77bcf7ce10fa91cc3689c0d11b0f4bm.
A backwards compatible approach would have added a response keyword
argument.  Unfortunately, that ship has sailed, unless we want to break
backwards compatibility again.

Try to restore backwards compatibility and align with standard Ruby
exception behavior by checking whether the first message to #initialize
is an STMP::Response.  If so, use it as the response.  If not, treat it
as the exception message.

This prevents issues in code that does something like:

```ruby
  raise exception_class, exception_message
```

Before this change, you'll get a NoMethodError later inside of
SMTPError#message, with no indication of the actual problem.
  • Loading branch information
jeremyevans committed May 16, 2022
1 parent 0b45b8e commit 3abed21
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions lib/net/smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,13 @@ module SMTPError
attr_reader :response

def initialize(response, message: nil)
@response = response
@message = message
if response.is_a?(::Net::SMTP::Response)
@response = response
@message = message
else
@response = nil
@message = message || response
end
end

def message
Expand Down Expand Up @@ -643,7 +648,7 @@ def do_start(helo_domain, user, secret, authtype)
do_helo helo_domain
if ! tls? and (starttls_always? or (capable_starttls? and starttls_auto?))
unless capable_starttls?
raise SMTPUnsupportedCommand.new(nil, message: "STARTTLS is not supported on this server")
raise SMTPUnsupportedCommand, "STARTTLS is not supported on this server"
end
starttls
@socket = new_internet_message_io(tlsconnect(s, @ssl_context_starttls))
Expand Down

1 comment on commit 3abed21

@KjellMorgenstern
Copy link

Choose a reason for hiding this comment

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

Thank you for this fix. This had given me a headache until I used this workaround in ApplicationMailer:

class ApplicationMailer < ActionMailer::Base
 rescue_from Net::SMTPAuthenticationError, with: :handle_smpt_delivery_error

 def handle_smpt_delivery_error(e)
    # After updating to ruby 3.1.2, SMTPError seems
    # to use 'response' instead of 'message' ?
    handle_delivery_error(e, e.response)
 end

After updating to net-smtp 0.3.2, error handling works without this workaround.

Please sign in to comment.