Skip to content

CXX-2705: Throw an exception upon unsuccessful pool creation #987

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
merged 10 commits into from
Jul 14, 2023
14 changes: 13 additions & 1 deletion src/mongocxx/pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@
namespace mongocxx {
MONGOCXX_INLINE_NAMESPACE_BEGIN

// Attempts to create a new client pool using the uri. Throws an exception upon error.
static mongoc_client_pool_t* construct_client_pool(mongoc_uri_t* uri) {
bson_error_t error;
auto pool = libmongoc::client_pool_new_with_error(uri, &error);
if (!pool) {
// If constructing a client pool failed, throw an exception from the bson_error_t.
throw_exception<operation_exception>(error);
}

return pool;
}

void pool::_release(client* client) {
libmongoc::client_pool_push(_impl->client_pool_t, client->_get_impl().client_t);
// prevent client destructor from destroying the underlying mongoc_client_t
Expand All @@ -43,7 +55,7 @@ void pool::_release(client* client) {
pool::~pool() = default;

pool::pool(const uri& uri, const options::pool& options)
: _impl{stdx::make_unique<impl>(libmongoc::client_pool_new(uri._impl->uri_t))} {
: _impl{stdx::make_unique<impl>(construct_client_pool(uri._impl->uri_t))} {
#if defined(MONGOCXX_ENABLE_SSL) && defined(MONGOC_ENABLE_SSL)
if (options.client_opts().tls_opts()) {
if (!uri.tls())
Expand Down
1 change: 1 addition & 0 deletions src/mongocxx/private/libmongoc_symbols.hh
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ MONGOCXX_LIBMONGOC_SYMBOL(client_new_from_uri)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_enable_auto_encryption)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_destroy)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_new)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_new_with_error)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_pop)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_push)
MONGOCXX_LIBMONGOC_SYMBOL(client_pool_set_apm_callbacks)
Expand Down
26 changes: 17 additions & 9 deletions src/mongocxx/test/pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "helpers.hpp"
#include <bsoncxx/test_util/catch.hh>
#include <mongocxx/client.hpp>
#include <mongocxx/exception/operation_exception.hpp>
#include <mongocxx/instance.hpp>
#include <mongocxx/options/tls.hpp>
#include <mongocxx/pool.hpp>
Expand All @@ -34,17 +35,18 @@ TEST_CASE("a pool is created with the correct MongoDB URI", "[pool]") {
instance::current();

bool destroy_called = false;
client_pool_destroy->interpose([&](::mongoc_client_pool_t*) { destroy_called = true; });
client_pool_destroy->visit([&](::mongoc_client_pool_t*) { destroy_called = true; });

std::string expected_uri("mongodb://mongodb.example.com:9999");
uri mongodb_uri{expected_uri};

std::string actual_uri{};
bool new_called = false;

client_pool_new->interpose([&](const mongoc_uri_t* uri) {
client_pool_new_with_error->visit([&](const mongoc_uri_t* uri, bson_error_t* error) {
new_called = true;
actual_uri = mongoc_uri_get_string(uri);
error->code = 0;
return nullptr;
});

Expand Down Expand Up @@ -87,11 +89,10 @@ TEST_CASE(

::mongoc_ssl_opt_t interposed = {};

client_pool_set_ssl_opts->interpose(
[&](::mongoc_client_pool_t*, const ::mongoc_ssl_opt_t* opts) {
set_tls_opts_called = true;
interposed = *opts;
});
client_pool_set_ssl_opts->visit([&](::mongoc_client_pool_t*, const ::mongoc_ssl_opt_t* opts) {
set_tls_opts_called = true;
interposed = *opts;
});

pool p{uri{"mongodb://mongodb.example.com:9999/?tls=true"},
options::client().tls_opts(tls_opts)};
Expand All @@ -112,13 +113,13 @@ TEST_CASE("calling acquire on a pool returns an entry that manages its client",
instance::current();

bool pop_called = false;
client_pool_pop->interpose([&](::mongoc_client_pool_t*) {
client_pool_pop->visit([&](::mongoc_client_pool_t*) {
pop_called = true;
return nullptr;
});

bool push_called = false;
client_pool_push->interpose(
client_pool_push->visit(
[&](::mongoc_client_pool_t*, ::mongoc_client_t*) { push_called = true; });

SECTION("entry releases its client at end of scope") {
Expand Down Expand Up @@ -167,4 +168,11 @@ TEST_CASE(
REQUIRE(!client);
}
}

TEST_CASE("a pool is created with an invalid connection string", "[pool]") {
instance::current();
std::string uristr = "mongodb+srv://foo.bar.baz";

REQUIRE_THROWS_AS(pool{mongocxx::uri(uristr)}, operation_exception);
}
} // namespace
7 changes: 1 addition & 6 deletions src/third_party/catch/include/helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,11 @@ MONGOCXX_INLINE_NAMESPACE_END
}

#define MOCK_POOL_NOSSL \
auto client_pool_new = libmongoc::client_pool_new.create_instance(); \
client_pool_new->interpose([](const mongoc_uri_t*) { return nullptr; }).forever(); \
auto client_pool_new_with_error = libmongoc::client_pool_new_with_error.create_instance(); \
auto client_pool_destroy = libmongoc::client_pool_destroy.create_instance(); \
client_pool_destroy->interpose([&](::mongoc_client_pool_t*) {}).forever(); \
auto client_pool_pop = libmongoc::client_pool_pop.create_instance(); \
client_pool_pop->interpose([](::mongoc_client_pool_t*) { return nullptr; }).forever(); \
auto client_pool_push = libmongoc::client_pool_push.create_instance(); \
client_pool_push->interpose([](::mongoc_client_pool_t*, ::mongoc_client_t*) {}).forever(); \
auto client_pool_try_pop = libmongoc::client_pool_try_pop.create_instance(); \
client_pool_try_pop->interpose([](::mongoc_client_pool_t*) { return nullptr; }).forever();

#if defined(MONGOCXX_ENABLE_SSL) && defined(MONGOC_ENABLE_SSL)
#define MOCK_POOL \
Expand Down