Skip to content

Commit cad8292

Browse files
jmikolajmikola
committed
PHPC-1373: Accessible WriteResult for executeBulkWrite socket error
executeBulkWrite() will now throw a BulkWriteException on top of any previous exception to ensure that a WriteResult can be attached. InvalidArgumentException is excluded, since that is only thrown before mongoc_bulk_operation_execute does IO (e.g. batch is empty).
1 parent abbabaf commit cad8292

10 files changed

+211
-24
lines changed

‎php_phongo.c‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -758,17 +758,41 @@ bool phongo_execute_bulk_write(mongoc_client_t* client, const char* namespace, p
758758
writeresult = phongo_writeresult_init(return_value, &reply, client, mongoc_bulk_operation_get_hint(bulk) TSRMLS_CC);
759759
writeresult->write_concern = mongoc_write_concern_copy(write_concern);
760760

761-
/* The Write failed */
761+
/* A BulkWriteException is always thrown if mongoc_bulk_operation_execute()
762+
* fails to ensure that the write result is accessible. If the error does
763+
* not originate from the server (e.g. socket error), throw the appropriate
764+
* exception first. It will be included in BulkWriteException's message and
765+
* will also be accessible via Exception::getPrevious(). */
762766
if (!success) {
763-
if (error.domain == MONGOC_ERROR_SERVER || error.domain == MONGOC_ERROR_WRITE_CONCERN) {
764-
zend_throw_exception(php_phongo_bulkwriteexception_ce, error.message, error.code TSRMLS_CC);
765-
phongo_exception_add_error_labels(&reply TSRMLS_CC);
766-
phongo_add_exception_prop(ZEND_STRL("writeResult"), return_value TSRMLS_CC);
767-
} else {
767+
if (error.domain != MONGOC_ERROR_SERVER && error.domain != MONGOC_ERROR_WRITE_CONCERN) {
768768
phongo_throw_exception_from_bson_error_and_reply_t(&error, &reply TSRMLS_CC);
769769
}
770+
771+
/* Argument errors occur before command execution, so there is no need
772+
* to layer this InvalidArgumentException behind a BulkWriteException.
773+
* In practice, this will be a "Cannot do an empty bulk write" error. */
774+
if (error.domain == MONGOC_ERROR_COMMAND && error.code == MONGOC_ERROR_COMMAND_INVALID_ARG) {
775+
goto cleanup;
776+
}
777+
778+
if (EG(exception)) {
779+
char *message;
780+
781+
(void) spprintf(&message, 0, "Bulk write failed due to previous %s: %s", PHONGO_ZVAL_EXCEPTION_NAME(EG(exception)), error.message);
782+
zend_throw_exception(php_phongo_bulkwriteexception_ce, message, 0 TSRMLS_CC);
783+
efree(message);
784+
} else {
785+
zend_throw_exception(php_phongo_bulkwriteexception_ce, error.message, error.code TSRMLS_CC);
786+
}
787+
788+
/* Ensure error labels are added to the final BulkWriteException. If a
789+
* previous exception was also thrown, error labels will already have
790+
* been added by phongo_throw_exception_from_bson_error_and_reply_t. */
791+
phongo_exception_add_error_labels(&reply TSRMLS_CC);
792+
phongo_add_exception_prop(ZEND_STRL("writeResult"), return_value TSRMLS_CC);
770793
}
771794

795+
cleanup:
772796
bson_destroy(&reply);
773797

774798
return success;

‎php_phongo.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,12 @@ zend_bool phongo_writeconcernerror_init(zval* return_value, bson_t* bson TSRMLS_
212212
#define PHONGO_ZVAL_CLASS_OR_TYPE_NAME(zv) (Z_TYPE(zv) == IS_OBJECT ? ZSTR_VAL(Z_OBJCE(zv)->name) : zend_get_type_by_const(Z_TYPE(zv)))
213213
#define PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(zvp) PHONGO_ZVAL_CLASS_OR_TYPE_NAME(*(zvp))
214214

215+
#if PHP_VERSION_ID >= 70000
216+
#define PHONGO_ZVAL_EXCEPTION_NAME(e) (ZSTR_VAL(e->ce->name))
217+
#else
218+
#define PHONGO_ZVAL_EXCEPTION_NAME(e) (ZSTR_VAL(Z_OBJCE_P(e)->name))
219+
#endif
220+
215221
#endif /* PHONGO_H */
216222

217223
/*

‎tests/bulk/bulkwrite-delete_error-004.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ $bulk->delete(
1919

2020
echo throws(function() use ($manager, $bulk) {
2121
$manager->executeBulkWrite(DATABASE_NAME . '.' . COLLECTION_NAME, $bulk);
22-
}, 'MongoDB\Driver\Exception\RuntimeException'), "\n";
22+
}, 'MongoDB\Driver\Exception\BulkWriteException'), "\n";
2323

2424
?>
2525
===DONE===
2626
<?php exit(0); ?>
2727
--EXPECT--
28-
OK: Got MongoDB\Driver\Exception\RuntimeException
29-
The selected server does not support collation
28+
OK: Got MongoDB\Driver\Exception\BulkWriteException
29+
Bulk write failed due to previous MongoDB\Driver\Exception\RuntimeException: The selected server does not support collation
3030
===DONE===

‎tests/bulk/bulkwrite-update_error-006.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ $bulk->update(
2020

2121
echo throws(function() use ($manager, $bulk) {
2222
$manager->executeBulkWrite(DATABASE_NAME . '.' . COLLECTION_NAME, $bulk);
23-
}, 'MongoDB\Driver\Exception\RuntimeException'), "\n";
23+
}, 'MongoDB\Driver\Exception\BulkWriteException'), "\n";
2424

2525
?>
2626
===DONE===
2727
<?php exit(0); ?>
2828
--EXPECT--
29-
OK: Got MongoDB\Driver\Exception\RuntimeException
30-
The selected server does not support collation
29+
OK: Got MongoDB\Driver\Exception\BulkWriteException
30+
Bulk write failed due to previous MongoDB\Driver\Exception\RuntimeException: The selected server does not support collation
3131
===DONE===

‎tests/bulk/bulkwrite-update_error-007.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ $bulk->update(
2323

2424
echo throws(function() use ($manager, $bulk) {
2525
$manager->executeBulkWrite(DATABASE_NAME . '.' . COLLECTION_NAME, $bulk);
26-
}, 'MongoDB\Driver\Exception\RuntimeException'), "\n";
26+
}, 'MongoDB\Driver\Exception\BulkWriteException'), "\n";
2727

2828
?>
2929
===DONE===
3030
<?php exit(0); ?>
3131
--EXPECT--
32-
OK: Got MongoDB\Driver\Exception\RuntimeException
33-
The selected server does not support array filters
32+
OK: Got MongoDB\Driver\Exception\BulkWriteException
33+
Bulk write failed due to previous MongoDB\Driver\Exception\RuntimeException: The selected server does not support array filters
3434
===DONE===

‎tests/connect/standalone-auth_error-001.phpt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@ $dsn = sprintf("mongodb://%s:%s@%s:%d/%s", $username, $password, $parsed["host"]
1818
$manager = new MongoDB\Driver\Manager($dsn);
1919

2020
$bulk = new MongoDB\Driver\BulkWrite;
21-
2221
$bulk->insert(array("my" => "value"));
23-
throws(function() use($manager, $bulk) {
24-
$retval = $manager->executeBulkWrite(NS, $bulk);
25-
}, "MongoDB\Driver\Exception\AuthenticationException");
22+
23+
echo throws(function() use($manager, $bulk) {
24+
$manager->executeBulkWrite(NS, $bulk);
25+
}, 'MongoDB\Driver\Exception\BulkWriteException'), "\n";
2626

2727
?>
2828
===DONE===
2929
<?php exit(0); ?>
3030
--EXPECT--
31-
OK: Got MongoDB\Driver\Exception\AuthenticationException
31+
OK: Got MongoDB\Driver\Exception\BulkWriteException
32+
Bulk write failed due to previous MongoDB\Driver\Exception\AuthenticationException: Authentication failed.
3233
===DONE===

‎tests/manager/manager-executeBulkWrite_error-005.phpt‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWrite() WriteResult accessible for network error
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_no_failcommand_failpoint(); ?>
7+
<?php skip_if_multiple_mongos(); ?>
8+
<?php skip_if_not_clean(); ?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = new MongoDB\Driver\Manager(URI);
14+
15+
configureFailPoint($manager, 'failCommand', [ 'times' => 1 ], [
16+
'failCommands' => ['delete'],
17+
'closeConnection' => true,
18+
]);
19+
20+
$bulk = new MongoDB\Driver\BulkWrite();
21+
$bulk->insert(['x' => 1]);
22+
$bulk->update(['x' => 1], ['$set' => ['y' => 1]]);
23+
$bulk->delete(['x' => 1]);
24+
25+
try {
26+
$manager->executeBulkWrite(NS, $bulk);
27+
} catch (MongoDB\Driver\Exception\BulkWriteException $e) {
28+
printf("%s(%d): %s\n", get_class($e), $e->getCode(), $e->getMessage());
29+
$prev = $e->getPrevious();
30+
printf("%s(%d): %s\n", get_class($prev), $prev->getCode(), $prev->getMessage());
31+
var_dump($e->getWriteResult());
32+
}
33+
34+
?>
35+
===DONE===
36+
<?php exit(0); ?>
37+
--EXPECTF--
38+
MongoDB\Driver\Exception\BulkWriteException(0): Bulk write failed due to previous MongoDB\Driver\Exception\ConnectionTimeoutException: Failed to send "delete" command with database "%s": Failed to read 4 bytes: socket error or timeout
39+
MongoDB\Driver\Exception\ConnectionTimeoutException(%d): Failed to send "delete" command with database "%s": Failed to read 4 bytes: socket error or timeout
40+
object(MongoDB\Driver\WriteResult)#%d (9) {
41+
["nInserted"]=>
42+
int(1)
43+
["nMatched"]=>
44+
int(1)
45+
["nModified"]=>
46+
int(1)
47+
["nRemoved"]=>
48+
int(0)
49+
["nUpserted"]=>
50+
int(0)
51+
["upsertedIds"]=>
52+
array(0) {
53+
}
54+
["writeErrors"]=>
55+
array(0) {
56+
}
57+
["writeConcernError"]=>
58+
NULL
59+
["writeConcern"]=>
60+
object(MongoDB\Driver\WriteConcern)#%d (0) {
61+
}
62+
}
63+
===DONE===

‎tests/manager/manager-executeBulkWrite_error-011.phpt‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWrite() BulkWriteException inherits labels from previous exception
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_libmongoc_crypto(); ?>
6+
<?php skip_if_not_replica_set(); ?>
7+
<?php skip_if_server_version('<', '4.0'); ?>
8+
<?php skip_if_no_failcommand_failpoint(); ?>
9+
<?php skip_if_not_clean(); ?>
10+
--FILE--
11+
<?php
12+
require_once __DIR__ . "/../utils/basic.inc";
13+
14+
$manager = new MongoDB\Driver\Manager(URI);
15+
16+
// Create collection since it can't be (automatically) done within the transaction
17+
$majority = new MongoDB\Driver\WriteConcern(MongoDB\Driver\WriteConcern::MAJORITY);
18+
$manager->executeWriteCommand(
19+
DATABASE_NAME,
20+
new MongoDB\Driver\Command(['create' => COLLECTION_NAME]),
21+
['writeConcern' => $majority]
22+
);
23+
24+
configureFailPoint($manager, 'failCommand', [ 'times' => 1 ], [
25+
'failCommands' => ['insert'],
26+
'closeConnection' => true,
27+
]);
28+
29+
$session = $manager->startSession();
30+
$session->startTransaction();
31+
32+
$bulk = new MongoDB\Driver\BulkWrite;
33+
$bulk->insert(['x' => 1]);
34+
35+
try {
36+
$manager->executeBulkWrite(NS, $bulk, ['session' => $session]);
37+
} catch (MongoDB\Driver\Exception\BulkWriteException $e) {
38+
printf("%s(%d): %s\n", get_class($e), $e->getCode(), $e->getMessage());
39+
var_dump($e->hasErrorLabel('TransientTransactionError'));
40+
$prev = $e->getPrevious();
41+
printf("%s(%d): %s\n", get_class($prev), $prev->getCode(), $prev->getMessage());
42+
var_dump($prev->hasErrorLabel('TransientTransactionError'));
43+
}
44+
45+
?>
46+
===DONE===
47+
<?php exit(0); ?>
48+
--EXPECTF--
49+
MongoDB\Driver\Exception\BulkWriteException(0): Bulk write failed due to previous MongoDB\Driver\Exception\ConnectionTimeoutException: Failed to send "insert" command with database "%s": Failed to read 4 bytes: socket error or timeout
50+
bool(true)
51+
MongoDB\Driver\Exception\ConnectionTimeoutException(%d): Failed to send "insert" command with database "%s": Failed to read 4 bytes: socket error or timeout
52+
bool(true)
53+
===DONE===

‎tests/manager/manager-set-uri-options-001.phpt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,18 @@ printf("Inserted: %d\n", $inserted);
3131
$options["username"] = "not-found-user";
3232
$manager = new MongoDB\Driver\Manager($dsn, $options);
3333
$bulk = new MongoDB\Driver\BulkWrite;
34-
3534
$bulk->insert(array("my" => "value"));
36-
throws(function() use ($manager, $bulk) {
35+
36+
echo throws(function() use ($manager, $bulk) {
3737
$inserted = $manager->executeBulkWrite(NS, $bulk)->getInsertedCount();
3838
printf("Incorrectly inserted: %d\n", $inserted);
39-
}, "MongoDB\Driver\Exception\AuthenticationException");
39+
}, 'MongoDB\Driver\Exception\BulkWriteException'), "\n";
40+
4041
?>
4142
===DONE===
4243
<?php exit(0); ?>
4344
--EXPECTF--
4445
Inserted: 1
45-
OK: Got MongoDB\Driver\Exception\AuthenticationException
46+
OK: Got MongoDB\Driver\Exception\BulkWriteException
47+
Bulk write failed due to previous MongoDB\Driver\Exception\AuthenticationException: Authentication failed.
4648
===DONE===

‎tests/utils/skipif.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
<?php
22

33
use MongoDB\Driver\Command;
4+
use MongoDB\Driver\Manager;
5+
use MongoDB\Driver\ReadPreference;
6+
use MongoDB\Driver\Server;
47
use MongoDB\Driver\Exception\ConnectionException;
58
use MongoDB\Driver\Exception\RuntimeException;
69

@@ -15,6 +18,28 @@ function skip_if_mongos()
1518
is_mongos(URI) and exit('skip topology is a sharded cluster');
1619
}
1720

21+
/**
22+
* Skips the test if the topology contains multiple mongos nodes.
23+
*
24+
* This is particularly useful for tests that rely on configureFailPoint, since
25+
* randomized server selection can interfere with testing.
26+
*/
27+
function skip_if_multiple_mongos()
28+
{
29+
$manager = new Manager(URI);
30+
31+
// Ensure SDAM is initialized before calling Manager::getServers()
32+
$manager->selectServer(new ReadPreference('nearest'));
33+
34+
$mongosNodes = array_filter($manager->getServers(), function(Server $server) {
35+
return $server->getType() === Server::TYPE_MONGOS;
36+
});
37+
38+
if (count($mongosNodes) > 1) {
39+
exit('skip topology contains multiple mongos nodes');
40+
}
41+
}
42+
1843
/**
1944
* Skips the test if the topology is not a shard cluster.
2045
*/
@@ -318,3 +343,16 @@ function skip_if_no_getmore_failpoint()
318343
exit("skip Server version '$serverVersion' does not support a getMore failpoint'");
319344
}
320345
}
346+
347+
function skip_if_no_failcommand_failpoint()
348+
{
349+
skip_if_test_commands_disabled();
350+
351+
$serverVersion = get_server_version(URI);
352+
353+
if (is_mongos(URI) && version_compare($serverVersion, '4.1.8', '<')) {
354+
exit("skip mongos version '$serverVersion' does not support 'failCommand' failpoint'");
355+
} elseif (version_compare($serverVersion, '4.0', '<')) {
356+
exit("skip mongod version '$serverVersion' does not support 'failCommand' failpoint'");
357+
}
358+
}

0 commit comments

Comments
 (0)