feat: Make error throttling smarter by factoring in error message (#287)

Author: limbonaut

CHANGELOG.md

@@ -7,6 +7,10 @@
 - Intoduce `SentryTimestamp` class to improve timestamp handling ([#286](https://github.com/getsentry/sentry-godot/pull/286))
 - Support for iOS & macOS using Sentry Cocoa SDK integration ([#266](https://github.com/getsentry/sentry-godot/pull/266))
 
+### Improvements
+
+- Make error throttling smarter by factoring in error message ([#287](https://github.com/getsentry/sentry-godot/pull/287))
+
 ## 1.0.0-alpha.3
 
 ### Breaking changes

src/sentry/sentry_logger.cpp

@@ -160,10 +160,36 @@ Vector<SentryEvent::StackFrame> _extract_error_stack_frames_from_backtraces(
 	return frames;
 }
 
+template <class T>
+inline size_t _hash(const T &p_value) {
+	std::hash<T> hasher;
+	return hasher(p_value);
+}
+
+template <class T>
+inline void _hash_combine(std::size_t &p_hash, const T &p_value) {
+	// NOTE: Hash combining technique, originally from boost.
+	std::hash<T> hasher;
+	p_hash ^= hasher(p_value) + 0x9e3779b9 + (p_hash << 6) + (p_hash >> 2);
+}
+
 } // unnamed namespace
 
 namespace sentry {
 
+std::size_t SentryLogger::ErrorKeyHash::operator()(const ErrorKey &p_key) const {
+	CharString message_cstr = p_key.message.utf8();
+	CharString filename_cstr = p_key.file.utf8();
+
+	std::string_view message_sv{ message_cstr.get_data() };
+	std::string_view filename_sv{ filename_cstr.get_data() };
+
+	size_t hash_value = _hash(message_sv);
+	_hash_combine(hash_value, filename_sv);
+	_hash_combine(hash_value, p_key.line);
+	return hash_value;
+}
+
 void SentryLogger::_connect_process_frame() {
 	SceneTree *scene_tree = Object::cast_to<SceneTree>(Engine::get_singleton()->get_main_loop());
 	if (scene_tree) {
@@ -198,8 +224,8 @@ void SentryLogger::_process_frame() {
 	}
 
 	// Clear source_line_times if it's too big. Cheap and efficient.
-	if (unlikely(source_line_times.size() > 100)) {
-		source_line_times.clear();
+	if (unlikely(error_timepoints.size() > 100)) {
+		error_timepoints.clear();
 	}
 }
 
@@ -214,7 +240,12 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 		return;
 	}
 
-	SourceLine source_line{ p_file.utf8(), p_line };
+	String error_message = p_rationale.is_empty() ? p_code : p_rationale;
+
+	ErrorKey error_key;
+	error_key.message = error_message;
+	error_key.file = p_file;
+	error_key.line = p_line;
 
 	TimePoint now = std::chrono::high_resolution_clock::now();
 
@@ -227,8 +258,8 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 		// Reject errors based on per-source-line throttling window to prevent
 		// repetitive logging caused by loops or errors recurring in each frame.
 		// The timestamps are tracked for each source line that produced an error.
-		auto it = source_line_times.find(source_line);
-		bool is_spammy_error = it != source_line_times.end() && now - it->second < limits.repeated_error_window;
+		auto it = error_timepoints.find(error_key);
+		bool is_spammy_error = it != error_timepoints.end() && now - it->second < limits.repeated_error_window;
 
 		bool within_frame_limit = frame_events < limits.events_per_frame;
 		bool within_throttling_limit = event_times.size() < limits.throttle_events;
@@ -248,7 +279,7 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 
 		if (as_event || as_breadcrumb) {
 			// Store timestamp to prevent repetitive logging from the same line of code.
-			source_line_times[source_line] = now;
+			error_timepoints[error_key] = now;
 		}
 	}
 
@@ -257,10 +288,8 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 		return;
 	}
 
-	String error_value = p_rationale.is_empty() ? p_code : p_rationale;
-
 	sentry::util::print_debug(
-			"Capturing error: ", error_value,
+			"Capturing error: ", error_message,
 			"\n   at: ", p_function, " (", p_file, ":", p_line, ")",
 			"\n   event: ", as_event, "  breadcrumb: ", as_breadcrumb);
 
@@ -282,7 +311,7 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 		ev->set_level(sentry::get_sentry_level_for_godot_error_type((GodotErrorType)p_error_type));
 		SentryEvent::Exception exception = {
 			error_type_as_string[int(p_error_type)],
-			error_value,
+			error_message,
 			frames
 		};
 		ev->add_exception(exception);
@@ -300,7 +329,7 @@ void SentryLogger::_log_error(const String &p_function, const String &p_file, in
 		data["error_type"] = String(error_type_as_string[int(p_error_type)]);
 
 		SentrySDK::get_singleton()->add_breadcrumb(
-				error_value,
+				error_message,
 				"error",
 				sentry::get_sentry_level_for_godot_error_type((GodotErrorType)p_error_type),
 				"error",

src/sentry/sentry_logger.h

@@ -18,27 +18,34 @@ class SentryLogger : public Logger {
 	GDCLASS(SentryLogger, Logger);
 
 private:
+	using GodotErrorType = sentry::GodotErrorType;
+	using TimePoint = std::chrono::high_resolution_clock::time_point;
+
 	struct Limits {
 		int events_per_frame;
 		std::chrono::milliseconds repeated_error_window;
 		std::chrono::milliseconds throttle_window;
 		int throttle_events;
 	} limits;
 
-	std::mutex error_mutex;
+	struct ErrorKey {
+		String message;
+		String file;
+		int line;
 
-	using GodotErrorType = sentry::GodotErrorType;
-	using SourceLine = std::pair<std::string, int>;
-	using TimePoint = std::chrono::high_resolution_clock::time_point;
-
-	struct SourceLineHash {
-		std::size_t operator()(const SourceLine &p_source_line) const {
-			return std::hash<std::string>()(p_source_line.first) ^ std::hash<int>()(p_source_line.second);
+		bool operator==(const ErrorKey &p_other) const {
+			return message == p_other.message && file == p_other.file && line == p_other.line;
 		}
 	};
 
+	struct ErrorKeyHash {
+		std::size_t operator()(const ErrorKey &p_key) const;
+	};
+
+	std::mutex error_mutex;
+
 	// Stores the last time an error was logged for each source line that generated an error.
-	std::unordered_map<SourceLine, TimePoint, SourceLineHash> source_line_times;
+	std::unordered_map<ErrorKey, TimePoint, ErrorKeyHash> error_timepoints;
 
 	// Time points for events captured within throttling window.
 	std::deque<TimePoint> event_times;