chore: Refactor logging & avoid logger errors caused by SDK debug printing

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Improvements
+
+- Detect when we're inside message logging to prevent SDK print operations through the Godot logger which cause runtime errors. ([#414](https://github.com/getsentry/sentry-godot/pull/414))
+
 ## 1.0.0
 
 ### Breaking changes

SConstruct

@@ -162,6 +162,7 @@ env.Append(CPPPATH=["src/"])
 sources = Glob("src/*.cpp")
 sources += Glob("src/editor/*.cpp")
 sources += Glob("src/sentry/*.cpp")
+sources += Glob("src/sentry/logging/*.cpp")
 sources += Glob("src/sentry/processing/*.cpp")
 sources += Glob("src/sentry/util/*.cpp")
 

project/test/suites/test_logger_integration.gd

@@ -1,5 +1,5 @@
 extends SentryTestSuite
-## Test SentryLogger error capture with detailed JSON validation.
+## Test SentryGodotLogger error capture with detailed JSON validation.
 ## NOTE: Additional logger tests can be found in test/isolated/ with filenames beginning with "test_logger_".
 
 
@@ -17,7 +17,7 @@ func test_gdscript_error_event_structure() -> void:
 
 	assert_json(json).describe("Logger event has logger attribute") \
 		.at("/") \
-		.must_contain("logger", "SentryLogger") \
+		.must_contain("logger", "SentryGodotLogger") \
 		.verify()
 
 	assert_json(json).describe("Logger error event contains exception data") \

src/editor/sentry_editor_export_plugin_unix.cpp

@@ -2,7 +2,7 @@
 
 #if defined(TOOLS_ENABLED) && !defined(WINDOWS_ENABLED)
 
-#include "sentry/util/print.h"
+#include "sentry/logging/print.h"
 
 #include <godot_cpp/classes/dir_access.hpp>
 #include <godot_cpp/classes/file_access.hpp>
@@ -25,7 +25,7 @@ void _set_executable_permissions(const String &p_path) {
 	if (perm != new_perm) {
 		Error err = FileAccess::set_unix_permissions(p_path, new_perm);
 		if (err != OK && err != ERR_UNAVAILABLE) {
-			sentry::util::print_warning("Failed to set executable permissions for: " + p_path);
+			sentry::logging::print_warning("Failed to set executable permissions for: " + p_path);
 		}
 	}
 }

src/editor/sentry_editor_plugin.cpp

@@ -4,12 +4,12 @@
 
 #include "editor/sentry_editor_export_plugin_android.h"
 #include "editor/sentry_editor_export_plugin_unix.h"
-#include "sentry/util/print.h"
+#include "sentry/logging/print.h"
 
 void SentryEditorPlugin::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_ENTER_TREE: {
-			sentry::util::print_debug("adding export plugins");
+			sentry::logging::print_debug("adding export plugins");
 
 			if (android_export_plugin.is_null()) {
 				android_export_plugin = Ref(memnew(SentryEditorExportPluginAndroid));

src/register_types.cpp

@@ -1,14 +1,14 @@
 #include "editor/sentry_editor_export_plugin_unix.h"
 #include "editor/sentry_editor_plugin.h"
 #include "sentry/disabled/disabled_event.h"
+#include "sentry/logging/sentry_godot_logger.h"
 #include "sentry/processing/screenshot_processor.h"
 #include "sentry/processing/sentry_event_processor.h"
 #include "sentry/processing/view_hierarchy_processor.h"
 #include "sentry/runtime_config.h"
 #include "sentry/sentry_attachment.h"
 #include "sentry/sentry_breadcrumb.h"
 #include "sentry/sentry_event.h"
-#include "sentry/sentry_logger.h"
 #include "sentry/sentry_options.h"
 #include "sentry/sentry_sdk.h"
 #include "sentry/sentry_user.h"
@@ -56,7 +56,7 @@ void register_runtime_classes() {
 	GDREGISTER_INTERNAL_CLASS(SentryEventProcessor);
 	GDREGISTER_INTERNAL_CLASS(ScreenshotProcessor);
 	GDREGISTER_INTERNAL_CLASS(ViewHierarchyProcessor);
-	GDREGISTER_INTERNAL_CLASS(SentryLogger);
+	GDREGISTER_INTERNAL_CLASS(logging::SentryGodotLogger);
 
 #ifdef SDK_NATIVE
 	GDREGISTER_INTERNAL_CLASS(native::NativeEvent);

src/sentry/android/android_sdk.cpp

@@ -5,9 +5,9 @@
 #include "android_string_names.h"
 #include "android_util.h"
 #include "sentry/common_defs.h"
+#include "sentry/logging/print.h"
 #include "sentry/processing/process_event.h"
 #include "sentry/sentry_attachment.h"
-#include "sentry/util/print.h"
 
 #include <godot_cpp/classes/engine.hpp>
 #include <godot_cpp/classes/project_settings.hpp>
@@ -22,7 +22,7 @@ void SentryAndroidBeforeSendHandler::_initialize(Object *p_android_plugin) {
 }
 
 void SentryAndroidBeforeSendHandler::_before_send(int32_t p_event_handle) {
-	sentry::util::print_debug("handling before_send: ", p_event_handle);
+	sentry::logging::print_debug("handling before_send: ", p_event_handle);
 
 	Ref<AndroidEvent> event_obj = memnew(AndroidEvent(android_plugin, p_event_handle));
 	event_obj->set_as_borrowed();
@@ -123,15 +123,15 @@ void AndroidSDK::add_attachment(const Ref<SentryAttachment> &p_attachment) {
 	ERR_FAIL_COND(p_attachment.is_null());
 
 	if (p_attachment->get_path().is_empty()) {
-		sentry::util::print_debug("attaching bytes with filename: ", p_attachment->get_filename());
+		sentry::logging::print_debug("attaching bytes with filename: ", p_attachment->get_filename());
 		android_plugin->call(ANDROID_SN(addBytesAttachment),
 				p_attachment->get_bytes(),
 				p_attachment->get_filename(),
 				p_attachment->get_content_type(),
 				String());
 	} else {
 		String absolute_path = ProjectSettings::get_singleton()->globalize_path(p_attachment->get_path());
-		sentry::util::print_debug("attaching file: ", absolute_path);
+		sentry::logging::print_debug("attaching file: ", absolute_path);
 		android_plugin->call(ANDROID_SN(addFileAttachment),
 				absolute_path,
 				p_attachment->get_filename(),

src/sentry/cocoa/cocoa_event.mm

@@ -1,7 +1,6 @@
 #include "cocoa_event.h"
 
 #include "cocoa_util.h"
-#include "sentry/util/print.h"
 
 #include <godot_cpp/classes/os.hpp>
 

src/sentry/cocoa/cocoa_sdk.mm

@@ -5,10 +5,10 @@
 #include "cocoa_includes.h"
 #include "cocoa_util.h"
 #include "sentry/common_defs.h"
+#include "sentry/logging/print.h"
 #include "sentry/processing/process_event.h"
 #include "sentry/sentry_attachment.h"
 #include "sentry/sentry_options.h"
-#include "sentry/util/print.h"
 
 #include <godot_cpp/classes/engine.hpp>
 #include <godot_cpp/classes/project_settings.hpp>
@@ -116,7 +116,7 @@
 		ERR_FAIL_NULL(ProjectSettings::get_singleton());
 		String absolute_path = ProjectSettings::get_singleton()->globalize_path(p_attachment->get_path());
 
-		sentry::util::print_debug(vformat("attaching file: %s", absolute_path));
+		sentry::logging::print_debug(vformat("attaching file: %s", absolute_path));
 
 		String filename = p_attachment->get_filename().is_empty() ? p_attachment->get_path().get_file() : p_attachment->get_filename();
 		attachment_objc = [[objc::SentryAttachment alloc] initWithPath:string_to_objc(absolute_path)
@@ -127,7 +127,7 @@
 		ERR_FAIL_COND_MSG(bytes.is_empty(), "Sentry: Can't add attachment with empty bytes and no file path.");
 		NSData *bytes_objc = [NSData dataWithBytes:bytes.ptr() length:bytes.size()];
 
-		sentry::util::print_debug(vformat("attaching bytes with filename: %s", p_attachment->get_filename()));
+		sentry::logging::print_debug(vformat("attaching bytes with filename: %s", p_attachment->get_filename()));
 
 		attachment_objc = [[objc::SentryAttachment alloc] initWithData:bytes_objc
 															  filename:string_to_objc(p_attachment->get_filename())
@@ -169,7 +169,7 @@
 		options.initialScope = ^(objc::SentryScope *scope) {
 			// Add global attachments
 			for (const String &path : p_global_attachments) {
-				sentry::util::print_debug("adding attachment \"", path, "\"");
+				sentry::logging::print_debug("adding attachment \"", path, "\"");
 				objc::SentryAttachment *att = nil;
 				if (path.ends_with(SENTRY_VIEW_HIERARCHY_FN)) {
 					// TODO: Can't specify attachmentType!

src/sentry/level.cpp

@@ -1,6 +1,6 @@
 #include "level.h"
 
-#include "sentry/util/print.h"
+#include "sentry/logging/print.h"
 
 namespace sentry {
 
@@ -38,7 +38,7 @@ Level int_to_level(int p_value) {
 		case 4:
 			return Level::LEVEL_FATAL;
 		default:
-			sentry::util::print_error("Internal Error: Unexpected SentryLevel integer value: " + godot::String::num_int64(p_value));
+			sentry::logging::print_error("Internal Error: Unexpected SentryLevel integer value: " + godot::String::num_int64(p_value));
 			return Level::LEVEL_ERROR;
 	}
 }

src/sentry/util/print.h → src/sentry/logging/print.h

@@ -2,17 +2,64 @@
 #define SENTRY_PRINT_H
 
 #include "sentry/level.h"
+#include "sentry/logging/state.h"
 #include "sentry/sentry_options.h"
 
+#include <cstdio>
 #include <godot_cpp/variant/utility_functions.hpp>
 #include <godot_cpp/variant/variant.hpp>
 
 using namespace godot;
 
-namespace sentry::util {
+namespace sentry::logging {
+
+template <typename... Args>
+void print_no_logger(sentry::Level p_level, const Variant &p_arg1, const Args &...p_args) {
+	if (!SentryOptions::get_singleton()->is_debug_enabled() && p_level < sentry::LEVEL_ERROR) {
+		return;
+	}
+	if (SentryOptions::get_singleton()->get_diagnostic_level() > p_level) {
+		return;
+	}
+
+	// Print level prefix
+	switch (p_level) {
+		case LEVEL_DEBUG: {
+			fputs("Sentry: DEBUG: ", stdout);
+		} break;
+		case LEVEL_INFO: {
+			fputs("Sentry: INFO: ", stdout);
+		} break;
+		case LEVEL_WARNING: {
+			fputs("Sentry: WARNING: ", stdout);
+		} break;
+		case LEVEL_ERROR: {
+			fputs("Sentry: ERROR: ", stdout);
+		} break;
+		case LEVEL_FATAL: {
+			fputs("Sentry: FATAL: ", stdout);
+		} break;
+	}
+
+	// Print first argument
+	fputs(Variant(p_arg1).stringify().utf8().get_data(), stdout);
+
+	// Print remaining arguments
+	(fputs(Variant(p_args).stringify().utf8().get_data(), stdout), ...);
+
+	fputc('\n', stdout);
+	fflush(stdout);
+}
 
 template <typename... Args>
 void print(sentry::Level p_level, const Variant &p_arg1, const Args &...p_args) {
+	if (sentry::logging::in_message_logging) {
+		// We shouldn't print anything to logger when another message is being logged,
+		// because it can cause runtime errors in Godot.
+		print_no_logger(p_level, p_arg1, p_args...);
+		return;
+	}
+
 	if (!SentryOptions::get_singleton()->is_debug_enabled() && p_level < sentry::LEVEL_ERROR) {
 		return;
 	}
@@ -66,16 +113,16 @@ void print_fatal(const Variant &p_arg1, const Args &...p_args) {
 
 #define FAIL_COND_V_PRINT_ERROR(m_cond, m_ret, m_msg) \
 	if (m_cond) {                                     \
-		sentry::util::print_error(m_msg);             \
+		sentry::logging::print_error(m_msg);          \
 		return m_ret;                                 \
 	}
 
 #define FAIL_COND_PRINT_ERROR(m_cond, m_msg) \
 	if (m_cond) {                            \
-		sentry::util::print_error(m_msg);    \
+		sentry::logging::print_error(m_msg); \
 		return;                              \
 	}
 
-} //namespace sentry::util
+} //namespace sentry::logging
 
 #endif // SENTRY_PRINT_H