RUBY-2509 Expose server error code name in OperationFailure exception message

 (#2189)

* RUBY-2509 Expose server error code name in OperationFailure exception message

* fix up the api docs

Co-authored-by: Oleg Pudeyev <oleg@bsdpower.com>

Author: p-mongo

lib/mongo/auth/base.rb

@@ -118,14 +118,14 @@ def dispatch_msg(connection, conversation, msg)
       # raises Unauthorized if not.
       def validate_reply!(connection, conversation, doc)
         if doc[:ok] != 1
-          extra = [doc[:code], doc[:codeName]].compact.join(': ')
-          msg = doc[:errmsg]
-          unless extra.empty?
-            msg += " (#{extra})"
-          end
+          message = Error::Parser.build_message(
+            code: doc[:code],
+            code_name: doc[:codeName],
+            message: doc[:errmsg],
+          )
           raise Unauthorized.new(user,
             used_mechanism: self.class.const_get(:MECHANISM),
-            message: msg,
+            message: message,
             server: connection.server,
           )
         end

lib/mongo/error.rb

@@ -29,16 +29,19 @@ class Error < StandardError
     # An error field, MongoDB < 2.6
     #
     # @since 2.0.0
+    # @deprecated
     ERR = '$err'.freeze
 
     # An error field, MongoDB < 2.4
     #
     # @since 2.0.0
+    # @deprecated
     ERROR = 'err'.freeze
 
     # The standard error message field, MongoDB 3.0+
     #
     # @since 2.0.0
+    # @deprecated
     ERRMSG = 'errmsg'.freeze
 
     # The constant for the writeErrors array.

lib/mongo/error/bulk_write_error.rb

@@ -15,9 +15,19 @@
 module Mongo
   class Error
 
-    # Exception raised if there are write errors upon executing the bulk
+    # Exception raised if there are write errors upon executing a bulk
     # operation.
     #
+    # Unlike OperationFailure, BulkWriteError does not currently expose
+    # individual error components (such as the error code). The result document
+    # (which can be obtained using the +result+ attribute) provides detailed
+    # error information and can be examined by the application if desired.
+    #
+    # @note A bulk operation that resulted in a BulkWriteError may have
+    #   written some of the documents to the database. If the bulk write
+    #   was unordered, writes may have also continued past the write that
+    #   produced a BulkWriteError.
+    #
     # @since 2.0.0
     class BulkWriteError < Error
 
@@ -47,11 +57,15 @@ def build_message
         return nil unless errors
 
         fragment = errors.first(10).map do |error|
-          "#{error['errmsg']} (#{error['code']})"
-        end.join(', ')
+          "[#{error['code']}]: #{error['errmsg']}"
+        end.join('; ')
 
         fragment += '...' if errors.length > 10
 
+        if errors.length > 1
+          fragment = "Multiple errors: #{fragment}"
+        end
+
         fragment
       end
     end

lib/mongo/error/operation_failure.rb

@@ -89,6 +89,12 @@ class OperationFailure < Error
       # @since 2.6.0
       attr_reader :code_name
 
+      # @return [ String ] The server-returned error message
+      #   parsed from the response.
+      #
+      # @api experimental
+      attr_reader :server_message
+
       # Whether the error is a retryable error according to the legacy
       # read retry logic.
       #
@@ -215,6 +221,11 @@ def write_concern_error?
       # @since 2.10.0
       attr_reader :write_concern_error_code_name
 
+      # @return [ BSON::Document | nil ] The server-returned error document.
+      #
+      # @api experimental
+      attr_reader :document
+
       # Create the operation failure.
       #
       # @example Create the error object
@@ -229,6 +240,10 @@ def write_concern_error?
       #
       # @option options [ Integer ] :code Error code.
       # @option options [ String ] :code_name Error code name.
+      # @option options [ BSON::Document ] :document The server-returned
+      #   error document.
+      # @option options [ String ] server_message The server-returned
+      #   error message parsed from the response.
       # @option options [ Hash ] :write_concern_error_document The
       #   server-supplied write concern error document, if any.
       # @option options [ Integer ] :write_concern_error_code Error code for
@@ -253,6 +268,8 @@ def initialize(message = nil, result = nil, options = {})
         @write_concern_error_labels = options[:write_concern_error_labels] || []
         @labels = options[:labels] || []
         @wtimeout = !!options[:wtimeout]
+        @document = options[:document]
+        @server_message = options[:server_message]
       end
 
       # Whether the error is a write concern timeout.
@@ -281,8 +298,10 @@ def max_time_ms_expired?
       #
       # @since 2.10.0
       def unsupported_retryable_write?
-        # code 20 is IllegalOperation
-        code == 20 && message.start_with?("Transaction numbers")
+        # code 20 is IllegalOperation.
+        # Note that the document is expected to be a BSON::Document, thus
+        # either having string keys or providing indifferent access.
+        code == 20 && server_message&.start_with?("Transaction numbers") || false
       end
     end
   end

lib/mongo/error/parser.rb

@@ -42,28 +42,53 @@ class Error
     # Class for parsing the various forms that errors can come in from MongoDB
     # command responses.
     #
+    # The errors can be reported by the server in a number of ways:
+    # - {ok:0} response indicates failure. In newer servers, code, codeName
+    #   and errmsg fields should be set. In older servers some may not be set.
+    # - {ok:1} response with a write concern error (writeConcernError top-level
+    #   field). This indicates that the node responding successfully executed
+    #   the request, but not enough other nodes successfully executed the
+    #   request to satisfy the write concern.
+    # - {ok:1} response with writeErrors top-level field. This can be obtained
+    #   in a bulk write but also in a non-bulk write. In a non-bulk write
+    #   there should be exactly one error in the writeErrors list.
+    #   The case of multiple errors is handled by BulkWrite::Result.
+    # - {ok:1} response with writeConcernErrors top-level field. This can
+    #   only be obtained in a bulk write and is handled by BulkWrite::Result,
+    #   not by this class.
+    #
+    # Note that writeErrors do not have codeName fields - they just provide
+    # codes and messages. writeConcernErrors may similarly not provide code
+    # names.
+    #
     # @since 2.0.0
+    # @api private
     class Parser
       include SdamErrorDetection
 
-      # @return [ BSON::Document ] document The returned document.
+      # @return [ BSON::Document ] The returned document.
       attr_reader :document
 
-      # @return [ String ] message The error message parsed from the document.
+      # @return [ String ] The full error message to be used in the
+      #   raised exception.
       attr_reader :message
 
-      # @return [ Array<Protocol::Message> ] replies The message replies.
+      # @return [ String ] The server-returned error message
+      #   parsed from the response.
+      attr_reader :server_message
+
+      # @return [ Array<Protocol::Message> ] The message replies.
       attr_reader :replies
 
-      # @return [ Integer ] code The error code parsed from the document.
+      # @return [ Integer ] The error code parsed from the document.
       # @since 2.6.0
       attr_reader :code
 
-      # @return [ String ] code_name The error code name parsed from the document.
+      # @return [ String ] The error code name parsed from the document.
       # @since 2.6.0
       attr_reader :code_name
 
-      # @return [ Array<String> ] labels The set of labels associated with the error.
+      # @return [ Array<String> ] The set of labels associated with the error.
       # @since 2.7.0
       attr_reader :labels
 
@@ -145,33 +170,61 @@ def write_concern_error_labels
         write_concern_error_document && write_concern_error_document['errorLabels']
       end
 
+      class << self
+        def build_message(code: nil, code_name: nil, message: nil)
+          if code_name && code
+            "[#{code}:#{code_name}]: #{message}"
+          elsif code_name
+            # This surely should never happen, if there's a code name
+            # there ought to also be the code provided.
+            # Handle this case for completeness.
+            "[#{code_name}]: #{message}"
+          elsif code
+            "[#{code}]: #{message}"
+          else
+            message
+          end
+        end
+      end
+
       private
 
       def parse!
+        if document['ok'] != 1 && document['writeErrors']
+          raise ArgumentError, "writeErrors should only be given in successful responses"
+        end
+
         @message = ""
-        parse_single(@message, ERR)
-        parse_single(@message, ERROR)
-        parse_single(@message, ERRMSG)
+        parse_single(@message, '$err')
+        parse_single(@message, 'err')
+        parse_single(@message, 'errmsg')
         parse_multiple(@message, 'writeErrors')
         if write_concern_error_document
-          parse_single(@message, ERRMSG, write_concern_error_document)
+          parse_single(@message, 'errmsg', write_concern_error_document)
         end
         parse_flag(@message)
         parse_code
         parse_labels
         parse_wtimeout
+
+        @server_message = @message
+        @message = self.class.build_message(
+          code: code,
+          code_name: code_name,
+          message: @message,
+        )
       end
 
       def parse_single(message, key, doc = document)
         if error = doc[key]
-          append(message ,"#{error} (#{doc[CODE]})")
+          append(message, error)
         end
       end
 
       def parse_multiple(message, key)
         if errors = document[key]
           errors.each do |error|
-            parse_single(message, ERRMSG, error)
+            parse_single(message, 'errmsg', error)
           end
         end
       end

lib/mongo/operation/list_collections/result.rb

@@ -88,7 +88,10 @@ def validate!
               code: parser.code,
               code_name: parser.code_name,
               labels: parser.labels,
-              wtimeout: parser.wtimeout)
+              wtimeout: parser.wtimeout,
+              document: parser.document,
+              server_message: parser.server_message,
+            )
           end
         end
 

lib/mongo/operation/result.rb

@@ -332,6 +332,8 @@ def error
           labels: parser.labels,
           wtimeout: parser.wtimeout,
           connection_description: connection_description,
+          document: parser.document,
+          server_message: parser.server_message,
         )
       end
 

lib/mongo/retryable.rb

@@ -477,7 +477,7 @@ def log_retry(e, options = nil)
       else
         "Retry"
       end
-      Logger.logger.warn "#{message} due to: #{e.class.name} #{e.message}"
+      Logger.logger.warn "#{message} due to: #{e.class.name}: #{e.message}"
     end
 
     # Retry writes on MMAPv1 should raise an actionable error; append actionable

spec/integration/bulk_write_error_message_spec.rb

@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe 'BulkWriteError message' do
+  let(:client) { authorized_client }
+  let(:collection_name) { 'bulk_write_error_message_spec' }
+  let(:collection) { client[collection_name] }
+
+  before do
+    collection.delete_many
+  end
+
+  context 'a bulk write with one error' do
+    it 'reports code name, code and message' do
+      begin
+        collection.insert_many([
+          {_id: 1},
+          {_id: 1},
+          {_id: 1},
+        ], ordered: true)
+        fail('Should have raised')
+      rescue Mongo::Error::BulkWriteError => e
+        e.message.should =~ %r,\A\[11000\]: (insertDocument :: caused by :: 11000 )?E11000 duplicate key error (collection|index):,
+      end
+    end
+  end
+
+  context 'a bulk write with multiple errors' do
+    it 'reports code name, code and message' do
+      begin
+        collection.insert_many([
+          {_id: 1},
+          {_id: 1},
+          {_id: 1},
+        ], ordered: false)
+        fail('Should have raised')
+      rescue Mongo::Error::BulkWriteError => e
+        e.message.should =~ %r,\AMultiple errors: \[11000\]: (insertDocument :: caused by :: 11000 )?E11000 duplicate key error (collection|index):.*\[11000\]: (insertDocument :: caused by :: 11000 )?E11000 duplicate key error (collection|index):,
+      end
+    end
+  end
+end

spec/integration/change_stream_spec.rb

@@ -98,7 +98,7 @@ def clear_fail_point_before
       it 'watch raises error' do
         expect do
           client['change-stream'].watch
-        end.to raise_error(Mongo::Error::OperationFailure, /Failing command due to 'failCommand' failpoint \(10107\)/)
+        end.to raise_error(Mongo::Error::OperationFailure, /10107\b.*Failing command due to 'failCommand' failpoint/)
       end
     end
 
@@ -283,7 +283,7 @@ def clear_fail_point_before
 
         expect do
           enum.next
-        end.to raise_error(Mongo::Error::OperationFailure, /Failing command due to 'failCommand' failpoint \(101\)/)
+        end.to raise_error(Mongo::Error::OperationFailure, /101\b.*Failing command due to 'failCommand' failpoint/)
       end
     end
   end
@@ -414,7 +414,7 @@ def clear_fail_point_before
 
           expect do
             enum.try_next
-          end.to raise_error(Mongo::Error::OperationFailure, /Failing command due to 'failCommand' failpoint \(10107\)/)
+          end.to raise_error(Mongo::Error::OperationFailure, /10107\b.*Failing command due to 'failCommand' failpoint/)
         end
       end
 
@@ -441,7 +441,7 @@ def clear_fail_point_before
 
           expect do
             enum.try_next
-          end.to raise_error(Mongo::Error::OperationFailure, /Failing command due to 'failCommand' failpoint \(10107\)/)
+          end.to raise_error(Mongo::Error::OperationFailure, /10107\b.*Failing command due to 'failCommand' failpoint/)
         end
       end
     end