Summary

The command monitoring spec defines a successful command as one with an ok:1 reply and a failed command as one with an ok:0 reply. However, it seems that at least the Node driver decides whether to emit a command succeeded or failed event based on whether or not an error is going to be thrown rather than the "ok" value.

While this seems reasonable, that logic is problematic because server replies can have ok:1 and a writeConcernError. In these cases, based on the definition of "success" in the command monitoring spec, we should emit a CommandSucceededEvent but also throw an error.

This came up because DRIVERS-2327 adds a prose test which sets a failpoint that results in an ok:1 response with a writeConcernError and then tells the driver to expect a command succeeded event. That prose test will implicitly cover this behavior but we should add a unified test to the command monitoring spec which explicitly covers it.

Motivation

Who is the affected end user?

Driver developers who use command monitoring events in tests; driver users who consume APM events.

How does this affect the end user?

For driver developers the bug is impactful as it leads to discrepancy in what events the driver emits based on certain errors which can lead to the driver failing spec tests.

Arguably this bug is less of a problem for driver users as they might expect that the buggy behavior is the right behavior (error = failed event).

How likely is it that this problem or use case will occur?

Any case where there is a writeConcernError and ok=1 (most write concern errors)

If the problem does occur, what are the consequences and how severe are they?

The wrong event type is emitted which could make e.g. logs generated from APM events confusing.

Is this issue urgent?

From a user facing perspective probably not, because we haven't heard about it yet from any users. That said, drivers will need to fix this bug to correctly implement DRIVERS-2327.

Is this ticket required by a downstream team?

No, but the Node driver having this bug would mean it also affects users of our dev tools like mongosh.

Is this ticket only for tests?

The only work here is to add a test, but this test would uncover a bug in drivers that do not pass it.