diff --git a/CHANGELOG.md b/CHANGELOG.md index d0f894ed0..bb3702950 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,15 +6,36 @@ - The `sentry_options_set_logger` function now accepts a `userdata` parameter. - The `name` parameter of `sentry_options_add_attachment(w)` was removed, it will now be inferred from the filename of `path`. -- The transport startup hook that is set via `sentry_transport_set_startup_func` now needs to return a `bool`, and a failure will propagate to `sentry_init`. +- The transport startup hook that is set via `sentry_transport_set_startup_func` now needs to return an `int`, and a failure will propagate to `sentry_init`. +- The return value of the transport shutdown hook set via `sentry_transport_set_shutdown_func` was also changed to return an `int`. +- Both functions should return _0_ on success, and a non-zero error code on failure, as does `sentry_init`. +- Similarly, the return value of `sentry_shutdown` was also changed to an `int`, and will return _0_ on success and a non-zero error code on unclean shutdown. ```c // before sentry_options_set_logger(options, my_custom_logger); sentry_options_add_attachment(options, "some-attachment", "/path/to/some-attachment.txt"); + +void transport_startup(sentry_options_t *options, void*state) { +} +sentry_transport_set_startup_func(transport, transport_startup); +bool transport_shutdown(uint64_t timeout, void*state) { + return true; +} +sentry_transport_set_shutdown_func(transport, transport_shutdown); + // after sentry_options_set_logger(options, my_custom_logger, NULL); sentry_options_add_attachment(options, "/path/to/some-attachment.txt"); + +int transport_startup(sentry_options_t *options, void*state) { + return 0; +} +sentry_transport_set_startup_func(transport, transport_startup); +int transport_shutdown(uint64_t timeout, void*state) { + return 0; +} +sentry_transport_set_shutdown_func(transport, transport_shutdown); ``` **Features**: diff --git a/examples/example.c b/examples/example.c index 929ae26a0..ccddca886 100644 --- a/examples/example.c +++ b/examples/example.c @@ -1,4 +1,5 @@ #include "sentry.h" +#include #include #include #include diff --git a/include/sentry.h b/include/sentry.h index cfe24aa77..3427bddb5 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -71,7 +71,6 @@ extern "C" { #include #include -#include #include /* context type dependencies */ @@ -561,22 +560,21 @@ SENTRY_API void sentry_transport_set_free_func( * * This hook is called from within `sentry_init` and will get a reference to the * options which can be used to initialize a transports internal state. - * Returning `false` from this hook will signal failure and will bubble up to - * `sentry_init`. + * It should return `0` on success. A failure will bubble up to `sentry_init`. */ SENTRY_API void sentry_transport_set_startup_func(sentry_transport_t *transport, - bool (*startup_func)(const sentry_options_t *options, void *state)); + int (*startup_func)(const sentry_options_t *options, void *state)); /** * Sets the transport shutdown hook. * - * This hook will receive a millisecond-resolution timeout; it should return - * `true` in case all the pending envelopes have been sent within the timeout, - * or `false` if the timeout was hit. + * This hook will receive a millisecond-resolution timeout. + * It should return `0` on success in case all the pending envelopes have been + * sent within the timeout, or `1` if the timeout was hit. */ SENTRY_API void sentry_transport_set_shutdown_func( sentry_transport_t *transport, - bool (*shutdown_func)(uint64_t timeout, void *state)); + int (*shutdown_func)(uint64_t timeout, void *state)); /** * Generic way to free a transport. @@ -885,13 +883,16 @@ SENTRY_API void sentry_options_set_system_crash_reporter_enabled( * they cannot be modified any more. * Depending on the configured transport and backend, this function might not be * fully thread-safe. + * Returns 0 on success. */ SENTRY_API int sentry_init(sentry_options_t *options); /** * Shuts down the sentry client and forces transports to flush out. + * + * Returns 0 on success. */ -SENTRY_API void sentry_shutdown(void); +SENTRY_API int sentry_shutdown(void); /** * Clears the internal module cache. diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 8fd5d96b8..38137a48b 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -176,7 +176,7 @@ sentry__breakpad_backend_callback( return succeeded; } -static bool +static int sentry__breakpad_backend_startup( sentry_backend_t *backend, const sentry_options_t *options) { diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index c8d8d9c9e..eaebc20bb 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -95,7 +95,7 @@ sentry__crashpad_backend_shutdown(sentry_backend_t *backend) data->db = nullptr; } -static bool +static int sentry__crashpad_backend_startup( sentry_backend_t *backend, const sentry_options_t *options) { @@ -130,7 +130,7 @@ sentry__crashpad_backend_startup( || !sentry__path_is_file(absolute_handler_path)) { SENTRY_WARN("unable to start crashpad backend, invalid handler_path"); sentry__path_free(absolute_handler_path); - return false; + return 1; } SENTRY_TRACEF("starting crashpad backend with handler " @@ -191,7 +191,7 @@ sentry__crashpad_backend_startup( // not calling `shutdown` delete data->db; data->db = nullptr; - return false; + return 1; } if (!options->system_crash_reporter_enabled) { @@ -202,7 +202,7 @@ sentry__crashpad_backend_startup( crashpad_info->set_system_crash_reporter_forwarding( crashpad::TriState::kDisabled); } - return true; + return 0; } static void diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 816cedd1e..9f048cdc0 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -71,7 +71,7 @@ invoke_signal_handler(int signum, siginfo_t *info, void *user_context) } } -static bool +static int startup_inproc_backend( sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options)) { @@ -81,14 +81,14 @@ startup_inproc_backend( if (sigaction( SIGNAL_DEFINITIONS[i].signum, NULL, &g_previous_handlers[i]) == -1) { - return false; + return 1; } } // install our own signal handler g_signal_stack.ss_sp = sentry_malloc(SIGNAL_STACK_SIZE); if (!g_signal_stack.ss_sp) { - return false; + return 1; } g_signal_stack.ss_size = SIGNAL_STACK_SIZE; g_signal_stack.ss_flags = 0; @@ -100,7 +100,7 @@ startup_inproc_backend( for (size_t i = 0; i < SIGNAL_COUNT; ++i) { sigaction(SIGNAL_DEFINITIONS[i].signum, &g_sigaction, NULL); } - return true; + return 0; } static void @@ -148,13 +148,13 @@ static const struct signal_slot SIGNAL_DEFINITIONS[SIGNAL_COUNT] = { static LONG WINAPI handle_exception(EXCEPTION_POINTERS *); -static bool +static int startup_inproc_backend( sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options)) { g_previous_handler = SetUnhandledExceptionFilter(&handle_exception); SetErrorMode(SEM_FAILCRITICALERRORS); - return true; + return 0; } static void diff --git a/src/sentry_backend.h b/src/sentry_backend.h index a2b3e9837..de395d67e 100644 --- a/src/sentry_backend.h +++ b/src/sentry_backend.h @@ -13,7 +13,7 @@ */ struct sentry_backend_s; typedef struct sentry_backend_s { - bool (*startup_func)( + int (*startup_func)( struct sentry_backend_s *, const sentry_options_t *options); void (*shutdown_func)(struct sentry_backend_s *); void (*free_func)(struct sentry_backend_s *); diff --git a/src/sentry_boot.h b/src/sentry_boot.h index b861950dc..3636f7806 100644 --- a/src/sentry_boot.h +++ b/src/sentry_boot.h @@ -34,5 +34,6 @@ #endif #include +#include #endif diff --git a/src/sentry_core.c b/src/sentry_core.c index 669433e64..24ec5c19e 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -97,7 +97,7 @@ sentry_init(sentry_options_t *options) load_user_consent(options); if (transport) { - if (!sentry__transport_startup(transport, options)) { + if (sentry__transport_startup(transport, options) != 0) { SENTRY_WARN("failed to initialize transport"); goto fail; } @@ -107,7 +107,7 @@ sentry_init(sentry_options_t *options) sentry_backend_t *backend = options->backend; if (backend && backend->startup_func) { SENTRY_TRACE("starting backend"); - if (!backend->startup_func(backend, options)) { + if (backend->startup_func(backend, options) != 0) { SENTRY_WARN("failed to initialize backend"); goto fail; } @@ -147,7 +147,7 @@ sentry_init(sentry_options_t *options) return 1; } -void +int sentry_shutdown(void) { sentry_end_session(); @@ -156,21 +156,22 @@ sentry_shutdown(void) sentry_options_t *options = g_options; sentry__mutex_unlock(&g_options_mutex); + size_t dumped_envelopes = 0; if (options) { - size_t dumped_envelopes = 0; + if (options->backend && options->backend->shutdown_func) { + SENTRY_TRACE("shutting down backend"); + options->backend->shutdown_func(options->backend); + } if (options->transport) { // TODO: make this configurable - if (!sentry__transport_shutdown( - options->transport, SENTRY_DEFAULT_SHUTDOWN_TIMEOUT)) { + if (sentry__transport_shutdown( + options->transport, SENTRY_DEFAULT_SHUTDOWN_TIMEOUT) + != 0) { SENTRY_WARN("transport did not shut down cleanly"); } dumped_envelopes = sentry__transport_dump_queue( options->transport, options->run); } - if (options->backend && options->backend->shutdown_func) { - SENTRY_TRACE("shutting down backend"); - options->backend->shutdown_func(options->backend); - } if (!dumped_envelopes) { sentry__run_clean(options->run); } @@ -182,6 +183,7 @@ sentry_shutdown(void) sentry__mutex_unlock(&g_options_mutex); sentry__scope_cleanup(); sentry__modulefinder_cleanup(); + return (int)dumped_envelopes; } void diff --git a/src/sentry_sync.c b/src/sentry_sync.c index e5fe6338d..6e154e961 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -194,7 +194,7 @@ worker_thread(void *data) return 0; } -void +int sentry__bgworker_start(sentry_bgworker_t *bgw) { SENTRY_TRACE("starting background worker thread"); @@ -204,7 +204,9 @@ sentry__bgworker_start(sentry_bgworker_t *bgw) if (sentry__thread_spawn(&bgw->thread_id, &worker_thread, bgw) != 0) { sentry__atomic_fetch_and_add(&bgw->running, -1); sentry__bgworker_decref(bgw); + return 1; } + return 0; } static void diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 3c8e9d724..b76429942 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -300,8 +300,9 @@ void sentry__bgworker_decref(sentry_bgworker_t *bgw); /** * Start a new background worker thread associated with `bgw`. + * Returns 0 on success. */ -void sentry__bgworker_start(sentry_bgworker_t *bgw); +int sentry__bgworker_start(sentry_bgworker_t *bgw); /** * This will try to shut down the background worker thread, with a `timeout`. diff --git a/src/sentry_transport.c b/src/sentry_transport.c index de798026e..953c4da72 100644 --- a/src/sentry_transport.c +++ b/src/sentry_transport.c @@ -10,12 +10,12 @@ typedef struct sentry_transport_s { void (*send_envelope_func)(sentry_envelope_t *envelope, void *state); - bool (*startup_func)(const sentry_options_t *options, void *state); - bool (*shutdown_func)(uint64_t timeout, void *state); + int (*startup_func)(const sentry_options_t *options, void *state); + int (*shutdown_func)(uint64_t timeout, void *state); void (*free_func)(void *state); size_t (*dump_func)(sentry_run_t *run, void *state); void *state; - bool was_started; + bool running; } sentry_transport_t; sentry_transport_t * @@ -45,14 +45,14 @@ sentry_transport_set_free_func( void sentry_transport_set_startup_func(sentry_transport_t *transport, - bool (*startup_func)(const sentry_options_t *options, void *state)) + int (*startup_func)(const sentry_options_t *options, void *state)) { transport->startup_func = startup_func; } void sentry_transport_set_shutdown_func(sentry_transport_t *transport, - bool (*shutdown_func)(uint64_t timeout, void *state)) + int (*shutdown_func)(uint64_t timeout, void *state)) { transport->shutdown_func = shutdown_func; } @@ -65,27 +65,28 @@ sentry__transport_send_envelope( transport->send_envelope_func(envelope, transport->state); } -bool +int sentry__transport_startup( sentry_transport_t *transport, const sentry_options_t *options) { if (transport->startup_func) { SENTRY_TRACE("starting transport"); - transport->was_started - = transport->startup_func(options, transport->state); - return transport->was_started; + int rv = transport->startup_func(options, transport->state); + transport->running = rv == 0; + return rv; } - return true; + return 0; } -bool +int sentry__transport_shutdown(sentry_transport_t *transport, uint64_t timeout) { - if (transport->shutdown_func && transport->was_started) { + if (transport->shutdown_func && transport->running) { SENTRY_TRACE("shutting down transport"); + transport->running = false; return transport->shutdown_func(timeout, transport->state); } - return true; + return 0; } void diff --git a/src/sentry_transport.h b/src/sentry_transport.h index 2e7d6780b..286fdad48 100644 --- a/src/sentry_transport.h +++ b/src/sentry_transport.h @@ -24,15 +24,18 @@ void sentry__transport_send_envelope( /** * Calls the transports startup hook. + * + * Returns 0 on success. */ -bool sentry__transport_startup( +int sentry__transport_startup( sentry_transport_t *transport, const sentry_options_t *options); /** * Instructs the transport to shut down. + * + * Returns 0 on success. */ -bool sentry__transport_shutdown( - sentry_transport_t *transport, uint64_t timeout); +int sentry__transport_shutdown(sentry_transport_t *transport, uint64_t timeout); /** * This will create a new platform specific HTTP transport. diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c index 6f0a2d6af..5740c1adf 100644 --- a/src/transports/sentry_transport_curl.c +++ b/src/transports/sentry_transport_curl.c @@ -55,7 +55,7 @@ sentry__curl_bgworker_state_free(void *_state) sentry_free(state); } -static bool +static int sentry__curl_transport_start( const sentry_options_t *options, void *transport_state) { @@ -64,7 +64,7 @@ sentry__curl_transport_start( CURLcode rv = curl_global_init(CURL_GLOBAL_ALL); if (rv != CURLE_OK) { SENTRY_WARNF("`curl_global_init` failed with code `%d`", (int)rv); - return false; + return 1; } } @@ -80,17 +80,16 @@ sentry__curl_transport_start( // In this case we don’t start the worker at all, which means we can // still dump all unsent envelopes to disk on shutdown. SENTRY_WARN("`curl_easy_init` failed"); - return false; + return 1; } - sentry__bgworker_start(bgworker); - return true; + return sentry__bgworker_start(bgworker); } -static bool +static int sentry__curl_transport_shutdown(uint64_t timeout, void *transport_state) { sentry_bgworker_t *bgworker = (sentry_bgworker_t *)transport_state; - return !sentry__bgworker_shutdown(bgworker, timeout); + return sentry__bgworker_shutdown(bgworker, timeout); } static size_t diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c index 6b267866c..4ebc0e0b6 100644 --- a/src/transports/sentry_transport_winhttp.c +++ b/src/transports/sentry_transport_winhttp.c @@ -42,15 +42,19 @@ static void sentry__winhttp_bgworker_state_free(void *_state) { winhttp_bgworker_state_t *state = _state; - WinHttpCloseHandle(state->connect); - WinHttpCloseHandle(state->session); + if (state->connect) { + WinHttpCloseHandle(state->connect); + } + if (state->session) { + WinHttpCloseHandle(state->session); + } sentry__rate_limiter_free(state->ratelimiter); sentry_free(state->user_agent); sentry_free(state->proxy); sentry_free(state); } -static bool +static int sentry__winhttp_transport_start( const sentry_options_t *opts, void *transport_state) { @@ -92,17 +96,16 @@ sentry__winhttp_transport_start( } if (!state->session) { SENTRY_WARN("`WinHttpOpen` failed"); - return false; + return 1; } - sentry__bgworker_start(bgworker); - return true; + return sentry__bgworker_start(bgworker); } -static bool +static int sentry__winhttp_transport_shutdown(uint64_t timeout, void *transport_state) { sentry_bgworker_t *bgworker = (sentry_bgworker_t *)transport_state; - return !sentry__bgworker_shutdown(bgworker, timeout); + return sentry__bgworker_shutdown(bgworker, timeout); } static void @@ -230,7 +233,9 @@ sentry__winhttp_send_task(void *_envelope, void *_state) SENTRY_TRACEF("request handled in %llums", now - started); exit: - WinHttpCloseHandle(request); + if (request) { + WinHttpCloseHandle(request); + } sentry_free(url); sentry_free(headers); sentry__prepared_http_request_free(req); diff --git a/tests/unit/test_failures.c b/tests/unit/test_failures.c index 3b65b375d..96483b316 100644 --- a/tests/unit/test_failures.c +++ b/tests/unit/test_failures.c @@ -2,11 +2,11 @@ #include "sentry_testsupport.h" #include -static bool +static int transport_startup_fail( const sentry_options_t *UNUSED(options), void *UNUSED(state)) { - return false; + return 1; } static void