Skip to content

Fixed PHPC-506: Use more descriptive messages in WriteExceptions #248

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion php_phongo.c
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,56 @@ mongoc_bulk_operation_t *phongo_bulkwrite_init(zend_bool ordered) { /* {{{ */
return mongoc_bulk_operation_new(ordered);
} /* }}} */

static void phongo_bulk_write_error_add_message(char **tmp_msg, bson_t *errors)
{
bson_iter_t iter;

bson_iter_init(&iter, errors);

while (bson_iter_next(&iter)) {
bson_t cbson;
uint32_t len;
const uint8_t *data;
bson_iter_t inner_iter;

if (!BSON_ITER_HOLDS_DOCUMENT(&iter)) {
continue;
}

bson_iter_document(&iter, &len, &data);

if (!bson_init_static(&cbson, data, len)) {
continue;
}

if (bson_iter_init_find(&inner_iter, &cbson, "errmsg") && BSON_ITER_HOLDS_UTF8(&inner_iter)) {
const char *tmp_errmsg = bson_iter_utf8(&inner_iter, NULL);
size_t tmp_errmsg_len = strlen(tmp_errmsg);

*tmp_msg = erealloc(*tmp_msg, strlen(*tmp_msg) + tmp_errmsg_len + 5);
strncpy(*tmp_msg + strlen(*tmp_msg), " :: ", 5);
strncpy(*tmp_msg + strlen(*tmp_msg), tmp_errmsg, tmp_errmsg_len + 1);
}
}
}

static char* phongo_assemble_bulk_write_error(mongoc_write_result_t *write_result)
{
char *tmp_msg = emalloc(sizeof("BulkWrite error"));

strncpy(tmp_msg, "BulkWrite error", sizeof("BulkWrite error"));

if (!bson_empty0(&write_result->writeErrors)) {
phongo_bulk_write_error_add_message(&tmp_msg, &write_result->writeErrors);
}

if (!bson_empty0(&write_result->writeConcernErrors)) {
phongo_bulk_write_error_add_message(&tmp_msg, &write_result->writeConcernErrors);
}

return tmp_msg;
}

bool phongo_execute_write(mongoc_client_t *client, const char *namespace, mongoc_bulk_operation_t *bulk, const mongoc_write_concern_t *write_concern, int server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
{
bson_error_t error;
Expand Down Expand Up @@ -576,7 +626,11 @@ bool phongo_execute_write(mongoc_client_t *client, const char *namespace, mongoc
/* FIXME: Maybe we can look at write_result.error and not pass error at all? */
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
} else {
phongo_throw_exception(PHONGO_ERROR_WRITE_FAILED TSRMLS_CC, "BulkWrite error");
char *bulk_error_msg;

bulk_error_msg = phongo_assemble_bulk_write_error(&writeresult->write_result);
phongo_throw_exception(PHONGO_ERROR_WRITE_FAILED TSRMLS_CC, "%s", bulk_error_msg);
efree(bulk_error_msg);
phongo_add_exception_prop(ZEND_STRL("writeResult"), return_value TSRMLS_CC);
}
return false;
Expand Down
4 changes: 2 additions & 2 deletions tests/manager/manager-executeBulkWrite-011.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ var_dump(iterator_to_array($cursor));
<?php exit(0); ?>
--EXPECTF--
OK: Got MongoDB\Driver\Exception\BulkWriteException
BulkWrite error
BulkWrite error :: Document failed validation
OK: Got MongoDB\Driver\Exception\BulkWriteException
BulkWrite error
BulkWrite error :: Document failed validation
array(3) {
[0]=>
object(stdClass)#%d (2) {
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var_dump(iterator_to_array($cursor));
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: E11000 duplicate key error %s: phongo.manager_manager_executeBulkWrite_error_001%sdup key: { : 1 }

===> WriteResult
server: %s:%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var_dump(iterator_to_array($cursor));
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: E11000 duplicate key error %s: phongo.manager_manager_executeBulkWrite_error_002%sdup key: { : 1 } :: E11000 duplicate key error %s: phongo.manager_manager_executeBulkWrite_error_002%sdup key: { : 2 }

===> WriteResult
server: %s:%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ var_dump(iterator_to_array($cursor));
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: Not enough data-bearing nodes

===> WriteResult
server: %s:%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-004.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var_dump(iterator_to_array($cursor));
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: unknown top level operator: $foo

===> WriteResult
server: %s:%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-005.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ try {
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: Document can't have $ prefixed field names: $foo

===> WriteResult
server: %s:%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-executeBulkWrite_error-006.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ try {
===DONE===
<?php exit(0); ?>
--EXPECTF--
BulkWriteException: BulkWrite error
BulkWriteException: BulkWrite error :: Unknown modifier: $foo

===> WriteResult
server: %s:%d
Expand Down