-
Type: Improvement
-
Resolution: Fixed
-
Priority: Major - P3
-
Affects Version/s: None
-
Component/s: libmongoc
-
None
The driver adds error labels for retryable writes as part of _mongoc_write_error_get_type per CDRIVER-3462.
In the review of CDRIVER-3462 I added some suggestions for refactoring this to be more consistent with existing code we have for adding error labels:
I wonder if it would make more sense to put this logic deeper in the command codepath.
For the network error check, we could follow suit with where a
TransientTransactionError label is added for network errors (see https://github.com/mongodb/specifications/blob/master/source/transactions/transactions.rst#error-reporting-changes).That's in the function "network_error_reply" of mongoc-cluster.c. It could append a RetryableWriteError if cmd->is_retryable_write is true.
That would also apply in more cases, which might be an existing bug in our retry logic. Because network_reply_error is called in cases like failure to decompress a server response (https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-cluster.c/#L3013) which sets an error domain MONGOC_ERROR_PROTOCOL (not MONGOC_ERROR_STREAM) but still calls network_error_reply.
For the server error check, _mongoc_write_error_is_retryable could be moved into mongoc-cluster.c and that check could be done around here: https://github.com/mongodb/mongo-c-driver/blob/1.16.0/src/libmongoc/src/mongoc/mongoc-cluster.c/#L3049
Though this is just a suggestion. A nice thing about adding the error label it here is that it keeps the retryable writes logic a bit more self contained. But I'm up to discuss this further.