-
Notifications
You must be signed in to change notification settings - Fork 245
DRIVERS-2871 CSOT tests: Expect withTransaction to propagate errors #1553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| command: | ||
| insert: *collectionName | ||
| maxTimeMS: { $$type: ["int", "long"] } | ||
| - commandStartedEvent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As withTransaction expects an error, then an abortTransaction event will be issued.
| expectError: | ||
| isClientError: true | ||
| expectError: | ||
| isClientError: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In PyMongo (and Go) the "expectError" on the insertOne above causes the test runner to catch, validate, and the ignore the exception so it never reaches the withTransaction block. So this new test would fail. Is it a bug that Java propagates the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. This was recently changed in https://jira.mongodb.org/browse/DRIVERS-1709 but wasn't documented in the UTR spec itself. The only reference is in the jira ticket:
Ensure that neither ignoreResultAndError nor expectError interfere with propagating exceptions from a callback operation to withTransaction. This came up with the Java driver's POC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I was looking at the old RST version. It is documented in the markdown version: https://github.com/mongodb/specifications/blob/master/source/unified-test-format/unified-test-format.md#withtransaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify was this change correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this change is correct given the new rules for handling errors in the UTR.
DRIVERS-2871
Please complete the following before merging:
clusters, and serverless).