From 2caea9b9f0ae08874faa343520d37b68f8ed961c Mon Sep 17 00:00:00 2001
From: Michael Klett <michael@webadvocate.com>
Date: Sat, 20 Nov 2010 11:50:10 -0500
Subject: [PATCH] RestClient::Exception#message should always return a String,
 never a NilClass.

RestClient::Exception subclasses RuntimeError, which is a subclass of Exception.  According to the Ruby documentation (http://www.ruby-doc.org/ruby-1.9/classes/Exception.html#M000100), the +message+ method is expected to return a string.  Before this commit, RestClient::Exception#message would return nil if a message was not set.

This presents a problem for any code that rescues a RestClient::Exception and expects a string in the +message+.

For example:

    begin
      raise RestClient::Exception
    rescue Exception => e
      puts e.message + "\n" + "More information"
    end

Or, for example, the real life code that uncovered this in production: https://github.com/collectiveidea/delayed_job/blob/a0d6cdb2634c3a1186a90c52df51c1fa7ae2ddab/lib/delayed/job.rb#L72

This fix makes RestClient::Exceptions better citizens of the Ruby ecosystem. :)
---
 history.md                   |  1 +
 lib/restclient/exceptions.rb | 12 ++++++++++--
 spec/exceptions_spec.rb      | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/history.md b/history.md
index 2d8e1ea..da134ec 100644
--- a/history.md
+++ b/history.md
@@ -6,6 +6,7 @@
 - block passing in Resource#[] (patch provided by Niko Dittmann)
 - cookies set in a response should be kept in a redirect
 - HEAD requests should process parameters just like GET (patch provided by Rob Eanes)
+- Exception message should never be nil (patch provided by Michael Klett)
 
 # 1.6.0
 
diff --git a/lib/restclient/exceptions.rb b/lib/restclient/exceptions.rb
index cbd35ea..7736125 100644
--- a/lib/restclient/exceptions.rb
+++ b/lib/restclient/exceptions.rb
@@ -80,7 +80,8 @@ module RestClient
   # For example, the entire result body (which is
   # probably an HTML error page) is e.response.
   class Exception < RuntimeError
-    attr_accessor :message, :response
+    attr_accessor :response
+    attr_writer   :message
 
     def initialize response = nil, initial_response_code = nil
       @response = response
@@ -110,6 +111,10 @@ module RestClient
     def to_s
       inspect
     end
+    
+    def message
+      @message || self.class.name
+    end
 
   end
 
@@ -162,7 +167,10 @@ module RestClient
   # this means it crashed, or sometimes that your network connection was
   # severed before it could complete.
   class ServerBrokeConnection < Exception
-    message = 'Server broke connection'
+    def initialize(message = 'Server broke connection')
+      super nil, nil
+      self.message = message
+    end
   end
 
   class SSLCertificateNotVerified < Exception
diff --git a/spec/exceptions_spec.rb b/spec/exceptions_spec.rb
index a4f511e..33342e3 100644
--- a/spec/exceptions_spec.rb
+++ b/spec/exceptions_spec.rb
@@ -4,6 +4,18 @@ require 'webmock/rspec'
 include WebMock
 
 describe RestClient::Exception do
+  it "returns a 'message' equal to the class name if the message is not set, because 'message' should not be nil" do
+    e = RestClient::Exception.new
+    e.message.should == "RestClient::Exception"
+  end
+  
+  it "returns the 'message' that was set" do
+    e = RestClient::Exception.new
+    message = "An explicitly set message"
+    e.message = message
+    e.message.should == message
+  end
+  
   it "sets the exception message to ErrorMessage" do
     RestClient::ResourceNotFound.new.message.should == 'Resource Not Found'
   end
@@ -14,6 +26,13 @@ describe RestClient::Exception do
   end
 end
 
+describe RestClient::ServerBrokeConnection do
+  it "should have a default message of 'Server broke connection'" do
+    e = RestClient::ServerBrokeConnection.new
+    e.message.should == 'Server broke connection'
+  end
+end
+
 describe RestClient::RequestFailed do
   before do
     @response = mock('HTTP Response', :code => '502')