From ff68a1ee0d50c07e1f80f18b232c759c3f785360 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Wed, 15 Nov 2023 12:53:55 +0100 Subject: [PATCH 1/9] DRIVERS-2584 Errors handling in Convenient Transactions API --- .../transactions-convenient-api.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index aac87bc37c..58310303b2 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -299,6 +299,18 @@ MongoClient within the callback in order to execute operations within the transaction. Per the `Driver Session`_ specification, ClientSessions should already provide access to a client object. +Errors handling inside the callback +----------------------------------- + +Drivers MUST document that the callback MUST NOT silently swallow errors that +do not have the "TransientTransactionError" label. Such errors abort the +transaction on the server; trying to commit aborted transaction will result in +an error. + +Drivers SHOULD recommend that the callback re-throw errors that do not have the +"TransientTransactionError" label or otherwise using Core Transaction API if +they want to handle errors in a custom way. + Test Plan ========= From eca77e8f9c9e42356754d30415a0b53be2cdc082 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Wed, 15 Nov 2023 13:39:43 +0100 Subject: [PATCH 2/9] Reword; add changelog entry --- .../transactions-convenient-api.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index 58310303b2..81e6171324 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -303,13 +303,13 @@ Errors handling inside the callback ----------------------------------- Drivers MUST document that the callback MUST NOT silently swallow errors that -do not have the "TransientTransactionError" label. Such errors abort the -transaction on the server; trying to commit aborted transaction will result in -an error. +do not have the "TransientTransactionError" label. Such errors abort +the transaction on the server; trying to commit an aborted transaction will +result in an error. -Drivers SHOULD recommend that the callback re-throw errors that do not have the -"TransientTransactionError" label or otherwise using Core Transaction API if -they want to handle errors in a custom way. +Drivers SHOULD recommend that the callback re-throw errors that do not have +the "TransientTransactionError" label. Drivers SHOULD also recommend using +Core Transaction API if a user wants to handle errors in a custom way. Test Plan ========= @@ -506,6 +506,7 @@ client-side operation timeout, withTransaction can continue to use the Changes ======= +:2023-11-15: Document error handling inside the callback. :2022-10-05: Remove spec front matter and reformat changelog. :2022-01-19: withTransaction applies timeouts per the client-side operations timeout specification. From 390455cce768edbb273fdf8ba12ef54fff2a9ac2 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Wed, 15 Nov 2023 15:30:54 +0100 Subject: [PATCH 3/9] Update source/transactions-convenient-api/transactions-convenient-api.rst Co-authored-by: Andreas Braun --- .../transactions-convenient-api/transactions-convenient-api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index 81e6171324..8e0b34d47c 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -299,7 +299,7 @@ MongoClient within the callback in order to execute operations within the transaction. Per the `Driver Session`_ specification, ClientSessions should already provide access to a client object. -Errors handling inside the callback +Handling errors inside the callback ----------------------------------- Drivers MUST document that the callback MUST NOT silently swallow errors that From 2e7408ea7fc8bb8ebf4f3cb23a740fdcab7270a3 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Wed, 15 Nov 2023 17:16:49 +0100 Subject: [PATCH 4/9] Fix code review remarks --- .../transactions-convenient-api.rst | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index 8e0b34d47c..80dfafb848 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -302,14 +302,22 @@ already provide access to a client object. Handling errors inside the callback ----------------------------------- -Drivers MUST document that the callback MUST NOT silently swallow errors that -do not have the "TransientTransactionError" label. Such errors abort -the transaction on the server; trying to commit an aborted transaction will -result in an error. +Drivers MUST document that the callback MUST NOT silently handle errors that +do not have the "TransientTransactionError" label without allowing such errors +to propagate. Such errors may abort the transaction on the server. + +For example, ``DuplicateKeyError`` is an error that aborts a transaction on the +server. If the callback catches ``DuplicateKeyError`` and does not re-throw it, +the driver will attempt to commit the transaction. The server will reject the +commit attempt with ``NoSuchTransaction`` error. This error has the +"TransientTransactionError" label and the driver will retry the commit. This +will result in an infinite loop. + Drivers SHOULD recommend that the callback re-throw errors that do not have -the "TransientTransactionError" label. Drivers SHOULD also recommend using -Core Transaction API if a user wants to handle errors in a custom way. +the "TransientTransactionError" label if they need to be handled inside the +callback. Drivers SHOULD also recommend using Core Transaction API if +a user wants to handle errors in a custom way. Test Plan ========= From 951f12f8d85f79fb1185fc3bec05c22d4cbe6e72 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Fri, 17 Nov 2023 15:09:01 +0100 Subject: [PATCH 5/9] Add section to transaction spec --- source/transactions/transactions.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source/transactions/transactions.rst b/source/transactions/transactions.rst index 7a576c2c01..f83b2953a8 100644 --- a/source/transactions/transactions.rst +++ b/source/transactions/transactions.rst @@ -982,6 +982,15 @@ label. For example: continue raise +Handling command errors +----------------------- + +Drivers MUST document that command errors that do not have the +"TransientTransactionError" label may abort the transaction on the server. +For example, ``DuplicateKeyError`` is an error that aborts a transaction; +an attempt to commit such transaction will be rejected with +``NoSuchTransaction`` error. + **Test Plan** ------------- From d49005e85c0e5070c28da18d3ad0c1c380eddea1 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Mon, 20 Nov 2023 13:20:21 +0100 Subject: [PATCH 6/9] Update changelogs --- .../transactions-convenient-api/transactions-convenient-api.rst | 2 +- source/transactions/transactions.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index 80dfafb848..ece2581b2f 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -514,7 +514,7 @@ client-side operation timeout, withTransaction can continue to use the Changes ======= -:2023-11-15: Document error handling inside the callback. +:2023-11-20: Document error handling inside the callback. :2022-10-05: Remove spec front matter and reformat changelog. :2022-01-19: withTransaction applies timeouts per the client-side operations timeout specification. diff --git a/source/transactions/transactions.rst b/source/transactions/transactions.rst index f83b2953a8..3f3c0eadf3 100644 --- a/source/transactions/transactions.rst +++ b/source/transactions/transactions.rst @@ -1416,6 +1416,8 @@ durable, which achieves the primary objective of avoiding duplicate commits. **Changelog** ------------- +:2023-11-20: Specify that non-transient transaction errors abort the transaction + on the server. :2022-10-05: Remove spec front matter and reformat changelog :2022-01-25: Mention the additional case of a retryable handshake error :2022-01-19: Deprecate maxCommitTimeMS in favor of timeoutMS. From f24d753e00aadd0294a56d571b4cc93af18f53a8 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Tue, 21 Nov 2023 09:38:43 +0100 Subject: [PATCH 7/9] Recommend to re-raise all errors --- .../transactions-convenient-api.rst | 14 +++++++------- source/transactions/transactions.rst | 8 +++----- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index ece2581b2f..cf0c335716 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -302,9 +302,10 @@ already provide access to a client object. Handling errors inside the callback ----------------------------------- -Drivers MUST document that the callback MUST NOT silently handle errors that -do not have the "TransientTransactionError" label without allowing such errors -to propagate. Such errors may abort the transaction on the server. +Drivers MUST document that the callback MUST NOT silently handle command errors +without allowing such errors to propagate. Command errors may abort the transaction +on the server, and an attempt to commit the transaction will be rejected with +``NoSuchTransaction`` error. For example, ``DuplicateKeyError`` is an error that aborts a transaction on the server. If the callback catches ``DuplicateKeyError`` and does not re-throw it, @@ -314,10 +315,9 @@ commit attempt with ``NoSuchTransaction`` error. This error has the will result in an infinite loop. -Drivers SHOULD recommend that the callback re-throw errors that do not have -the "TransientTransactionError" label if they need to be handled inside the -callback. Drivers SHOULD also recommend using Core Transaction API if -a user wants to handle errors in a custom way. +Drivers SHOULD recommend that the callback re-throw command errors if they +need to be handled inside the callback. Drivers SHOULD also recommend using +Core Transaction API if a user wants to handle errors in a custom way. Test Plan ========= diff --git a/source/transactions/transactions.rst b/source/transactions/transactions.rst index 3f3c0eadf3..dd99bee8cb 100644 --- a/source/transactions/transactions.rst +++ b/source/transactions/transactions.rst @@ -985,11 +985,9 @@ label. For example: Handling command errors ----------------------- -Drivers MUST document that command errors that do not have the -"TransientTransactionError" label may abort the transaction on the server. -For example, ``DuplicateKeyError`` is an error that aborts a transaction; -an attempt to commit such transaction will be rejected with -``NoSuchTransaction`` error. +Drivers MUST document that command errors inside a transaction may abort +the transaction on the server. An attempt to commit such transaction will be +rejected with ``NoSuchTransaction`` error. **Test Plan** ------------- From 07c8ca9a6b2f56d66dba44d687efeb469847cf29 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Fri, 24 Nov 2023 09:23:47 +0100 Subject: [PATCH 8/9] Use MSUT for recommendation --- .../transactions-convenient-api.rst | 4 ++-- source/transactions/transactions.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index cf0c335716..d5a73413ad 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -315,7 +315,7 @@ commit attempt with ``NoSuchTransaction`` error. This error has the will result in an infinite loop. -Drivers SHOULD recommend that the callback re-throw command errors if they +Drivers MUST recommend that the callback re-throw command errors if they need to be handled inside the callback. Drivers SHOULD also recommend using Core Transaction API if a user wants to handle errors in a custom way. @@ -514,7 +514,7 @@ client-side operation timeout, withTransaction can continue to use the Changes ======= -:2023-11-20: Document error handling inside the callback. +:2023-11-22: Document error handling inside the callback. :2022-10-05: Remove spec front matter and reformat changelog. :2022-01-19: withTransaction applies timeouts per the client-side operations timeout specification. diff --git a/source/transactions/transactions.rst b/source/transactions/transactions.rst index dd99bee8cb..d32763ae01 100644 --- a/source/transactions/transactions.rst +++ b/source/transactions/transactions.rst @@ -1414,7 +1414,7 @@ durable, which achieves the primary objective of avoiding duplicate commits. **Changelog** ------------- -:2023-11-20: Specify that non-transient transaction errors abort the transaction +:2023-11-22: Specify that non-transient transaction errors abort the transaction on the server. :2022-10-05: Remove spec front matter and reformat changelog :2022-01-25: Mention the additional case of a retryable handshake error From 015941034bd0358e5f08f3f9aeabf0d8a3f05f3b Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Tue, 28 Nov 2023 16:36:44 +0100 Subject: [PATCH 9/9] Update source/transactions-convenient-api/transactions-convenient-api.rst Co-authored-by: Jeremy Mikola --- .../transactions-convenient-api/transactions-convenient-api.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/source/transactions-convenient-api/transactions-convenient-api.rst b/source/transactions-convenient-api/transactions-convenient-api.rst index d5a73413ad..5fe349a896 100644 --- a/source/transactions-convenient-api/transactions-convenient-api.rst +++ b/source/transactions-convenient-api/transactions-convenient-api.rst @@ -314,7 +314,6 @@ commit attempt with ``NoSuchTransaction`` error. This error has the "TransientTransactionError" label and the driver will retry the commit. This will result in an infinite loop. - Drivers MUST recommend that the callback re-throw command errors if they need to be handled inside the callback. Drivers SHOULD also recommend using Core Transaction API if a user wants to handle errors in a custom way.