{"action":"created","issue":{"url":"https://api.github.com/repos/gabime/spdlog/issues/231","repository_url":"https://api.github.com/repos/gabime/spdlog","labels_url":"https://api.github.com/repos/gabime/spdlog/issues/231/labels{/name}","comments_url":"https://api.github.com/repos/gabime/spdlog/issues/231/comments","events_url":"https://api.github.com/repos/gabime/spdlog/issues/231/events","html_url":"https://github.com/gabime/spdlog/pull/231","id":161029680,"number":231,"title":"Allow custom error handling to avoid exceptions","user":{"login":"godbyk","id":195036,"avatar_url":"https://avatars.githubusercontent.com/u/195036?v=3","gravatar_id":"","url":"https://api.github.com/users/godbyk","html_url":"https://github.com/godbyk","followers_url":"https://api.github.com/users/godbyk/followers","following_url":"https://api.github.com/users/godbyk/following{/other_user}","gists_url":"https://api.github.com/users/godbyk/gists{/gist_id}","starred_url":"https://api.github.com/users/godbyk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/godbyk/subscriptions","organizations_url":"https://api.github.com/users/godbyk/orgs","repos_url":"https://api.github.com/users/godbyk/repos","events_url":"https://api.github.com/users/godbyk/events{/privacy}","received_events_url":"https://api.github.com/users/godbyk/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-06-18T15:31:48Z","updated_at":"2016-06-18T19:44:50Z","closed_at":"2016-06-18T16:18:38Z","pull_request":{"url":"https://api.github.com/repos/gabime/spdlog/pulls/231","html_url":"https://github.com/gabime/spdlog/pull/231","diff_url":"https://github.com/gabime/spdlog/pull/231.diff","patch_url":"https://github.com/gabime/spdlog/pull/231.patch"},"body":"(This PR is a duplicate of PR #230. It just targets the `no-exceptions` branch instead of the `master` branch.)\r\n\r\nThis PR addresses issue #136 by adding the ability to replace the default error handling (i.e., throwing a `spdlog_ex` exception) with a custom error handler (e.g., one that prints the error to `STDERR`).\r\n\r\n * All instances of `throw spdlog_ex` have been replaced with a call to the new `spdlog::error()` function (with one exception—see Further Work below).\r\n * The `spdlog::error()` function takes a `std::string` argument and passes it along to the error handler.\r\n * The default error handler simple throws a `spdlog_ex` exception containing the string as the message.\r\n * The user may provide their own error handler with the following signature: `void my_error_handler(const std::string& message)`. An example error handler may simply print the message to `STDERR`.\r\n\r\n## Further Work\r\n\r\n * The `mpmc_bounded_queue` constructor in the `details/mpmc_bounded_q.h` file still throws an exception because I wasn't sure how best to avoid it. If `buffer_size` isn't a power of two, can we round it down to the nearest power of two and avoid the error entirely?\r\n * Check to see where `fmt` may throw exceptions and ensure we catch them and pass through through the new error handler instead of allowing them to propagate up the stack.\r\n * Since the error handler may not throw an exception and interrupt the program flow, the library developers must assume that flow will continue after the error handler has been called. Therefore, they should take the appropriate action (e.g., return early, fall back on a sane default value). If there is no way to avoid throwing an exception, then the caller should wrap the function with a `try`...`catch` block to prevent the exception from propagating and call the error handler instead.\r\n"},"comment":{"url":"https://api.github.com/repos/gabime/spdlog/issues/comments/226961359","html_url":"https://github.com/gabime/spdlog/pull/231#issuecomment-226961359","issue_url":"https://api.github.com/repos/gabime/spdlog/issues/231","id":226961359,"user":{"login":"godbyk","id":195036,"avatar_url":"https://avatars.githubusercontent.com/u/195036?v=3","gravatar_id":"","url":"https://api.github.com/users/godbyk","html_url":"https://github.com/godbyk","followers_url":"https://api.github.com/users/godbyk/followers","following_url":"https://api.github.com/users/godbyk/following{/other_user}","gists_url":"https://api.github.com/users/godbyk/gists{/gist_id}","starred_url":"https://api.github.com/users/godbyk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/godbyk/subscriptions","organizations_url":"https://api.github.com/users/godbyk/orgs","repos_url":"https://api.github.com/users/godbyk/repos","events_url":"https://api.github.com/users/godbyk/events{/privacy}","received_events_url":"https://api.github.com/users/godbyk/received_events","type":"User","site_admin":false},"created_at":"2016-06-18T19:44:50Z","updated_at":"2016-06-18T19:44:50Z","body":"Using `-fno-exceptions` would complicate things a bit. I never use `-fno-exceptions`, so I'm not sure how this impacts the use of the standard library. **I think a good first iteration is just to ensure that spdlog can supply `nothrow` guarantees on as many functions as possible.**\r\n\r\nThe `FMT_TRY` macro is (currently) only used in the two internal error-handling functions `format_windows_error()` and `format_system_error()`. In both cases, any exceptions thrown within that try block are caught and swallowed (i.e., `catch (...) { /* do nothing */ }`). These functions call ``format_error_cocde()` which is marked `FMT_NOEXCEPT`.  **We definitely need to examine all the uses of `FMT_THROW`, however, and try to catch those exceptions within spdlog so they don't propagate out of spdlog.**\r\n\r\nFinally, as to returning after calling the error handler: I agree. I've done that in most of the cases already, but would very much appreciate someone reviewing them to ensure correctness. In some cases (e.g., the exception that was being thrown in the file rotation constructor) I fell back on sane defaults and allowed it to continue executing the remainder of the function. In other cases, I just returned early (trying to return a sensible value where possible) to prevent further execution of the code in that function. **But this needs to be evaluated on a case-by-case basis.**\r\n\r\n### Conclusion\r\n\r\nI think we should handle this in two phases:\r\n\r\n1. Ensure that spdlog never throws exceptions that a spdlog user must handle. This means that spdlog needs to add `try...catch` blocks around any calls to cpp-format that may throw exceptions and Do the Right Thing™ in handling those errors. We should mark as much as possible with no-throw exception guarantees and be explicit in any instances where we can uphold this guarantee.\r\n\r\n2. Support `-fno-exceptions`. This may impact our use of cpp-format (which may need to be modified or replaced) and the standard library. I suspect this will be more difficult to attain, but believe it is also of less importance than (1) above.\r\n\r\n\r\n"}}
{"actor":{"display_login":"godbyk"}}