SPEC-1505 Use a whitelist for classifying change stream errors (#736)

Author: Divjot Arora

source/change-streams/change-streams.rst

@@ -9,8 +9,8 @@ Change Streams
 :Status: Accepted
 :Type: Standards
 :Minimum Server Version: 3.6
-:Last Modified: April 3, 2019
-:Version: 1.7.0
+:Last Modified: Febrary 10, 2020
+:Version: 1.8.0
 
 .. contents::
 
@@ -43,35 +43,60 @@ Resumable Error
 
 An error is considered resumable if it meets any of the following criteria:
 
-- any error encountered which is not a server error (e.g. a timeout error or
+- Any error encountered which is not a server error (e.g. a timeout error or
   network error)
 
-- *any* server error response from a getMore command excluding those
-  containing the error label `NonResumableChangeStreamError` and those
-  containing the following error codes
+- For servers with wire version 9 or higher (server version 4.4 or higher), any
+  server error with the `ResumableChangeStreamError` error label.
+
+- For servers with wire version less than 9, a server error with one of the
+  following codes:
 
   .. list-table::
     :header-rows: 1
 
     * - Error Name
       - Error Code
-    * - Interrupted
-      - 11601
-    * - CappedPositionLost
-      - 136
-    * - CursorKilled
-      - 237
+    * - HostUnreachable
+      - 6
+    * - HostNotFound
+      - 7
+    * - NetworkTimeout
+      - 89
+    * - ShutdownInProgress
+      - 91
+    * - PrimarySteppedDown
+      - 189
+    * - ExceededTimeLimit
+      - 262
+    * - SocketException
+      - 9001
+    * - NotMaster
+      - 10107
+    * - InterruptedAtShutdown
+      - 11600
+    * - InterruptedDueToReplStateChange
+      - 11602
+    * - NotMasterNoSlaveOk
+      - 13435
+    * - NotMasterOrSecondary
+      - 13436
+    * - StaleShardVersion
+      - 63
+    * - StaleEpoch
+      - 150
+    * - StaleConfig
+      - 13388
+    * - RetryChangeStream
+      - 234
+    * - FailedToSatisfyReadPreference
+      - 133
+    * - ElectionInProgress
+      - 216
 
 An error on an aggregate command is not a resumable error. Only errors on a
 getMore command may be considered resumable errors.
 
-The criteria for resumable errors is similar to the discussion in the SDAM
-spec's section on `Error Handling`_, but includes additional error codes. See
-`What do the additional error codes mean?`_ for the reasoning behind these
-additional errors.
-
-.. _Error Handling: ../server-discovery-and-monitoring/server-discovery-and-monitoring.rst#error-handling
-
 --------
 Guidance
 --------
@@ -439,7 +464,7 @@ A change stream MUST track the last resume token, per `Updating the Cached Resum
 
 Drivers MUST raise an error on the first document received without a resume token (e.g. the user has removed ``_id`` with a pipeline stage), and close the change stream.  The error message SHOULD resemble “Cannot provide resume functionality when the resume token is missing”.
 
-A change stream MUST attempt to resume a single time if it encounters any resumable error.  A change stream MUST NOT attempt to resume on any other type of error, with the exception of a “not master” server error.  If a driver receives a “not master” error (for instance, because the primary it was connected to is stepping down), it will treat the error as a resumable error and attempt to resume.
+A change stream MUST attempt to resume a single time if it encounters any resumable error per `Resumable Error`_.  A change stream MUST NOT attempt to resume on any other type of error.
 
 In addition to tracking a resume token, change streams MUST also track the read preference specified when the change stream was created. In the event of a resumable error, a change stream MUST perform server selection with the original read preference before attempting to resume.
 
@@ -676,13 +701,14 @@ It was decided to remove this example from the specification for the following r
 - There is something to be said for an API that allows cooperation by default. The model in which a call to next only blocks until any response is returned (even an empty batch), allows for interruption and cooperation (e.g. interaction with other event loops).
 
 ----------------------------------------
-What do the additional error codes mean?
+Why is a whitelist of error codes preferable to a blacklist?
 ----------------------------------------
 
-The `CursorKilled` or `Interrupted` error implies some other actor killed the cursor.
-
-The `CappedPositionLost` error implies falling off of the back of the oplog,
-so resuming is impossible.
+Change streams originally used a blacklist of error codes to determine which errors were not resumable. However, this
+allowed for the possibility of infinite resume loops if an error was not correctly blacklisted. Due to the fact that
+all errors aside from transient issues such as failovers are not resumable, the resume behavior was changed to use a
+whitelist. Part of this change was to introduce the ``ResumableChangeStreamError`` label so the server can add new error
+codes to the whitelist without requiring changes to drivers.
 
 -------------------------------------------------------------------------------------------
 Why do we need to send a default ``startAtOperationTime`` when resuming a ``ChangeStream``?
@@ -795,3 +821,5 @@ Changelog
 | 2019-07-15 | Clarify resume process for change streams started with     |
 |            | the ``startAfter`` option.                                 |
 +------------+------------------------------------------------------------+
+| 2020-02-10 | Changed error handling approach to use a whitelist         |
++------------+------------------------------------------------------------+

source/change-streams/tests/README.rst

@@ -93,7 +93,10 @@ Spec Test Runner
 
 Before running the tests
 
-- Create a MongoClient ``globalClient``, and connect to the server
+- Create a MongoClient ``globalClient``, and connect to the server.
+When executing tests against a sharded cluster, ``globalClient`` must only connect to one mongos. This is because tests
+that set failpoints will only work consistently if both the ``configureFailPoint`` and failing commands are sent to the
+same mongos.
 
 For each YAML file, for each element in ``tests``:
 
@@ -128,8 +131,8 @@ For each YAML file, for each element in ``tests``:
 - If there are any ``expectations``
 
   - For each (``expected``, ``idx``) in ``expectations``
-
-    - Assert that ``actual[idx]`` MATCHES ``expected``
+    - If ``actual[idx]`` is a ``killCursors`` event, skip it and move to ``actual[idx+1]``.
+    - Else assert that ``actual[idx]`` MATCHES ``expected``
 
 - Close the MongoClient ``client``
 
@@ -155,12 +158,12 @@ The following tests have not yet been automated, but MUST still be tested. All t
 #. ``ChangeStream`` will throw an exception if the server response is missing the resume token (if wire version is < 8, this is a driver-side error; for 8+, this is a server-side error)
 #. After receiving a ``resumeToken``, ``ChangeStream`` will automatically resume one time on a resumable error with the initial pipeline and options, except for the addition/update of a ``resumeToken``.
 #. ``ChangeStream`` will not attempt to resume on any error encountered while executing an ``aggregate`` command. Note that retryable reads may retry ``aggregate`` commands. Drivers should be careful to distinguish retries from resume attempts. Alternatively, drivers may specify `retryReads=false` or avoid using a [retryable error](../../retryable-reads/retryable-reads.rst#retryable-error) for this test.
-#. ``ChangeStream`` will not attempt to resume after encountering error code 11601 (Interrupted), 136 (CappedPositionLost), or 237 (CursorKilled) while executing a ``getMore`` command.
+#. **Removed**
 #. ``ChangeStream`` will perform server selection before attempting to resume, using initial ``readPreference``
 #. Ensure that a cursor returned from an aggregate command with a cursor id and an initial empty batch is not closed on the driver side.
 #. The ``killCursors`` command sent during the "Resume Process" must not be allowed to throw an exception.
 #. ``$changeStream`` stage for ``ChangeStream`` against a server ``>=4.0`` and ``<4.0.7`` that has not received any results yet MUST include a ``startAtOperationTime`` option when resuming a change stream.
-#. ``ChangeStream`` will resume after a ``killCursors`` command is issued for its child cursor.
+#. **Removed**
 #. For a ``ChangeStream`` under these conditions:
 
    - Running against a server ``>=4.0.7``.

source/change-streams/tests/change-streams-errors.json

@@ -108,6 +108,53 @@
           ]
         }
       }
+    },
+    {
+      "description": "change stream errors on MaxTimeMSExpired",
+      "minServerVersion": "4.2",
+      "failPoint": {
+        "configureFailPoint": "failCommand",
+        "mode": {
+          "times": 1
+        },
+        "data": {
+          "failCommands": [
+            "getMore"
+          ],
+          "errorCode": 50,
+          "closeConnection": false
+        }
+      },
+      "target": "collection",
+      "topology": [
+        "replicaset",
+        "sharded"
+      ],
+      "changeStreamPipeline": [
+        {
+          "$project": {
+            "_id": 0
+          }
+        }
+      ],
+      "changeStreamOptions": {},
+      "operations": [
+        {
+          "database": "change-stream-tests",
+          "collection": "test",
+          "name": "insertOne",
+          "arguments": {
+            "document": {
+              "z": 3
+            }
+          }
+        }
+      ],
+      "result": {
+        "error": {
+          "code": 50
+        }
+      }
     }
   ]
 }

source/change-streams/tests/change-streams-errors.yml

@@ -73,3 +73,32 @@ tests:
       error:
         code: 280
         errorLabels: [ "NonResumableChangeStreamError" ]
+  -
+    description: change stream errors on MaxTimeMSExpired
+    minServerVersion: "4.2"
+    failPoint:
+      configureFailPoint: failCommand
+      mode: { times: 1 }
+      data:
+          failCommands: ["getMore"]
+          errorCode: 50 # An error code that's not on the old blacklist or whitelist
+          closeConnection: false
+    target: collection
+    topology:
+      - replicaset
+      - sharded
+    changeStreamPipeline:
+      -
+        $project: { _id: 0 }
+    changeStreamOptions: {}
+    operations:
+      -
+        database: *database_name
+        collection: *collection_name
+        name: insertOne
+        arguments:
+          document:
+            z: 3
+    result:
+      error:
+        code: 50