-
Notifications
You must be signed in to change notification settings - Fork 208
PHPC-1076: Expose result document for failed commands via CommandFailedEvent #840
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
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.
LGTM other than a change for the commandFailed test
int(9) | ||
["codeName"]=> | ||
string(13) "FailedToParse" | ||
} |
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.
This test fails because var_dump($event->getReply())
returns an object
that includes operationTime
, $clusterTime
, and signature
.
This can be fixed by adding
["operationTime"]=>
object(MongoDB\BSON\Timestamp)#%d (2) {
["increment"]=>
string(1) "1"
["timestamp"]=>
string(10) "%d"
}
["$clusterTime"]=>
object(stdClass)#%d (2) {
["clusterTime"]=>
object(MongoDB\BSON\Timestamp)#%d (2) {
["increment"]=>
string(1) "1"
["timestamp"]=>
string(10) "%d"
}
["signature"]=>
object(stdClass)#%d (2) {
["hash"]=>
object(MongoDB\BSON\Binary)#%d (2) {
["data"]=>
string(20) "%s"
["type"]=>
int(0)
}
["keyId"]=>
int(0)
}
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.
Thanks for catching that. It didn't come up with overview.php
because I was already using %A
there for 3.2 compatibility.
Note: Changes in #841 should address Windows builds. |
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.
It looks OK, but I'd prefer you would rebase this on master after merging #841, to make sure the Windows build is actually addressed.
MongoDB 3.4 introduced a codeName field to error replies, so we use a wildcard to ignore that for 3.2 servers.
3.6 replica sets may include additional fields in the command response (e.g. operationTime, $clusterTime).
https://jira.mongodb.org/browse/PHPC-1076
Supersedes #818. This includes the bump to libmongoc 1.10-dev and test suite changes for connection strings and skip functions.