Implement custom exception approach for errors from C code

Author: bouwkast

ext/datadog_profiling_native_extension/clock_id_from_pthread.c

@@ -18,7 +18,7 @@ void self_test_clock_id(void) {
   rb_nativethread_id_t expected_pthread_id = pthread_self();
   rb_nativethread_id_t actual_pthread_id = pthread_id_for(rb_thread_current());
 
-  if (expected_pthread_id != actual_pthread_id) rb_raise(rb_eRuntimeError, "pthread_id_for() self-test failed");
+  if (expected_pthread_id != actual_pthread_id) rb_raise(datadog_profiling_error_class, "pthread_id_for() self-test failed");
 }
 
 // Safety: This function is assumed never to raise exceptions by callers

ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c

@@ -289,7 +289,7 @@ void collectors_cpu_and_wall_time_worker_init(VALUE profiling_module) {
       after_gc_from_postponed_job_handle == POSTPONED_JOB_HANDLE_INVALID ||
       after_gvl_running_from_postponed_job_handle == POSTPONED_JOB_HANDLE_INVALID
     ) {
-      rb_raise(rb_eRuntimeError, "Failed to register profiler postponed jobs (got POSTPONED_JOB_HANDLE_INVALID)");
+      rb_raise(datadog_profiling_error_class, "Failed to register profiler postponed jobs (got POSTPONED_JOB_HANDLE_INVALID)");
     }
   #else
     gc_finalize_deferred_workaround = objspace_ptr_for_gc_finalize_deferred_workaround();
@@ -473,7 +473,7 @@ static VALUE _native_sampling_loop(DDTRACE_UNUSED VALUE _self, VALUE instance) {
   if (old_state != NULL) {
     if (is_thread_alive(old_state->owner_thread)) {
       rb_raise(
-        rb_eRuntimeError,
+        datadog_profiling_error_class,
         "Could not start CpuAndWallTimeWorker: There's already another instance of CpuAndWallTimeWorker active in a different thread"
       );
     } else {
@@ -1284,7 +1284,7 @@ static VALUE rescued_sample_allocation(DDTRACE_UNUSED VALUE unused) {
 
 static void delayed_error(cpu_and_wall_time_worker_state *state, const char *error) {
   // If we can't raise an immediate exception at the calling site, use the asynchronous flow through the main worker loop.
-  stop_state(state, rb_exc_new_cstr(rb_eRuntimeError, error));
+  stop_state(state, rb_exc_new_cstr(datadog_profiling_error_class, error));
 }
 
 static VALUE _native_delayed_error(DDTRACE_UNUSED VALUE self, VALUE instance, VALUE error_msg) {

ext/datadog_profiling_native_extension/collectors_discrete_dynamic_sampler.c

@@ -369,7 +369,7 @@ static VALUE _native_new(VALUE klass) {
 
   long now_ns = monotonic_wall_time_now_ns(DO_NOT_RAISE_ON_FAILURE);
   if (now_ns == 0) {
-    rb_raise(rb_eRuntimeError, "failed to get clock time");
+    rb_raise(datadog_profiling_error_class, "failed to get clock time");
   }
   discrete_dynamic_sampler_init(&state->sampler, "test sampler", now_ns);
 

ext/datadog_profiling_native_extension/collectors_gc_profiling_helper.c

@@ -119,7 +119,7 @@ uint8_t gc_profiling_set_metadata(ddog_prof_Label *labels, int labels_length) {
   };
 
   if (label_pos > max_label_count) {
-    rb_raise(rb_eRuntimeError, "BUG: gc_profiling_set_metadata unexpected label_pos (%d) > max_label_count (%d)", label_pos, max_label_count);
+    rb_raise(datadog_profiling_error_class, "BUG: gc_profiling_set_metadata unexpected label_pos (%d) > max_label_count (%d)", label_pos, max_label_count);
   }
 
   return label_pos;

ext/datadog_profiling_native_extension/collectors_idle_sampling_helper.c

@@ -153,7 +153,7 @@ static void *run_idle_sampling_loop(void *state_ptr) {
     // Process pending action
     if (next_action == ACTION_RUN) {
       if (run_action_function == NULL) {
-        grab_gvl_and_raise(rb_eRuntimeError, "Unexpected NULL run_action_function in run_idle_sampling_loop");
+        grab_gvl_and_raise(datadog_profiling_error_class, "Unexpected NULL run_action_function in run_idle_sampling_loop");
       }
 
       run_action_function();

ext/datadog_profiling_native_extension/collectors_stack.c

@@ -284,11 +284,11 @@ void sample_thread(
   // here, but >= 0 makes this easier to understand/debug.
   bool only_wall_time = cpu_or_wall_sample && values.cpu_time_ns == 0 && values.wall_time_ns >= 0;
 
-  if (cpu_or_wall_sample && state_label == NULL) rb_raise(rb_eRuntimeError, "BUG: Unexpected missing state_label");
+  if (cpu_or_wall_sample && state_label == NULL) rb_raise(datadog_profiling_error_class, "BUG: Unexpected missing state_label");
 
   if (has_cpu_time) {
     state_label->str = DDOG_CHARSLICE_C("had cpu");
-    if (labels.is_gvl_waiting_state) rb_raise(rb_eRuntimeError, "BUG: Unexpected combination of cpu-time with is_gvl_waiting");
+    if (labels.is_gvl_waiting_state) rb_raise(datadog_profiling_error_class, "BUG: Unexpected combination of cpu-time with is_gvl_waiting");
   }
 
   int top_of_stack_position = captured_frames - 1;

ext/datadog_profiling_native_extension/collectors_thread_context.c

@@ -831,7 +831,7 @@ VALUE thread_context_collector_sample_after_gc(VALUE self_instance) {
   TypedData_Get_Struct(self_instance, thread_context_collector_state, &thread_context_collector_typed_data, state);
 
   if (state->gc_tracking.wall_time_at_previous_gc_ns == INVALID_TIME) {
-    rb_raise(rb_eRuntimeError, "BUG: Unexpected call to sample_after_gc without valid GC information available");
+    rb_raise(datadog_profiling_error_class, "BUG: Unexpected call to sample_after_gc without valid GC information available");
   }
 
   int max_labels_needed_for_gc = 7; // Magic number gets validated inside gc_profiling_set_metadata
@@ -998,7 +998,7 @@ static void trigger_sample_for_thread(
   // @ivoanjo: I wonder if C compilers are smart enough to statically prove this check never triggers unless someone
   // changes the code erroneously and remove it entirely?
   if (label_pos > max_label_count) {
-    rb_raise(rb_eRuntimeError, "BUG: Unexpected label_pos (%d) > max_label_count (%d)", label_pos, max_label_count);
+    rb_raise(datadog_profiling_error_class, "BUG: Unexpected label_pos (%d) > max_label_count (%d)", label_pos, max_label_count);
   }
 
   ddog_prof_Slice_Label slice_labels = {.ptr = labels, .len = label_pos};
@@ -1295,7 +1295,7 @@ static long update_time_since_previous_sample(long *time_at_previous_sample_ns,
       elapsed_time_ns = 0;
     } else {
       // We don't expect non-wall time to go backwards, so let's flag this as a bug
-      rb_raise(rb_eRuntimeError, "BUG: Unexpected negative elapsed_time_ns between samples");
+      rb_raise(datadog_profiling_error_class, "BUG: Unexpected negative elapsed_time_ns between samples");
     }
   }
 
@@ -1961,7 +1961,7 @@ static uint64_t otel_span_id_to_uint(VALUE otel_span_id) {
     thread_context_collector_state *state;
     TypedData_Get_Struct(self_instance, thread_context_collector_state, &thread_context_collector_typed_data, state);
 
-    if (!state->timeline_enabled) rb_raise(rb_eRuntimeError, "GVL profiling requires timeline to be enabled");
+    if (!state->timeline_enabled) rb_raise(datadog_profiling_error_class, "GVL profiling requires timeline to be enabled");
 
     intptr_t gvl_waiting_at = gvl_profiling_state_thread_object_get(current_thread);
 

ext/datadog_profiling_native_extension/heap_recorder.c

@@ -300,7 +300,7 @@ void start_heap_allocation_recording(heap_recorder *heap_recorder, VALUE new_obj
   }
 
   if (heap_recorder->active_recording != NULL) {
-    rb_raise(rb_eRuntimeError, "Detected consecutive heap allocation recording starts without end.");
+    rb_raise(datadog_profiling_error_class, "Detected consecutive heap allocation recording starts without end.");
   }
 
   if (++heap_recorder->num_recordings_skipped < heap_recorder->sample_rate ||
@@ -323,7 +323,7 @@ void start_heap_allocation_recording(heap_recorder *heap_recorder, VALUE new_obj
 
   VALUE ruby_obj_id = rb_obj_id(new_obj);
   if (!FIXNUM_P(ruby_obj_id)) {
-    rb_raise(rb_eRuntimeError, "Detected a bignum object id. These are not supported by heap profiling.");
+    rb_raise(datadog_profiling_error_class, "Detected a bignum object id. These are not supported by heap profiling.");
   }
 
   heap_recorder->active_recording = object_record_new(
@@ -371,7 +371,7 @@ static VALUE end_heap_allocation_recording(VALUE protect_args) {
 
   if (active_recording == NULL) {
     // Recording ended without having been started?
-    rb_raise(rb_eRuntimeError, "Ended a heap recording that was not started");
+    rb_raise(datadog_profiling_error_class, "Ended a heap recording that was not started");
   }
   // From now on, mark the global active recording as invalid so we can short-circuit at any point
   // and not end up with a still active recording. the local active_recording still holds the
@@ -487,14 +487,14 @@ void heap_recorder_prepare_iteration(heap_recorder *heap_recorder) {
 
   if (heap_recorder->object_records_snapshot != NULL) {
     // we could trivially handle this but we raise to highlight and catch unexpected usages.
-    rb_raise(rb_eRuntimeError, "New heap recorder iteration prepared without the previous one having been finished.");
+    rb_raise(datadog_profiling_error_class, "New heap recorder iteration prepared without the previous one having been finished.");
   }
 
   heap_recorder_update(heap_recorder, /* full_update: */ true);
 
   heap_recorder->object_records_snapshot = st_copy(heap_recorder->object_records);
   if (heap_recorder->object_records_snapshot == NULL) {
-    rb_raise(rb_eRuntimeError, "Failed to create heap snapshot.");
+    rb_raise(datadog_profiling_error_class, "Failed to create heap snapshot.");
   }
 }
 
@@ -505,7 +505,7 @@ void heap_recorder_finish_iteration(heap_recorder *heap_recorder) {
 
   if (heap_recorder->object_records_snapshot == NULL) {
     // we could trivially handle this but we raise to highlight and catch unexpected usages.
-    rb_raise(rb_eRuntimeError, "Heap recorder iteration finished without having been prepared.");
+    rb_raise(datadog_profiling_error_class, "Heap recorder iteration finished without having been prepared.");
   }
 
   st_free_table(heap_recorder->object_records_snapshot);
@@ -733,19 +733,19 @@ static void commit_recording(heap_recorder *heap_recorder, heap_record *heap_rec
   // needed to fully build the object_record.
   active_recording->heap_record = heap_record;
   if (heap_record->num_tracked_objects == UINT32_MAX) {
-    rb_raise(rb_eRuntimeError, "Reached maximum number of tracked objects for heap record");
+    rb_raise(datadog_profiling_error_class, "Reached maximum number of tracked objects for heap record");
   }
   heap_record->num_tracked_objects++;
 
   int existing_error = st_update(heap_recorder->object_records, active_recording->obj_id, update_object_record_entry, (st_data_t) active_recording);
   if (existing_error) {
     object_record *existing_record = NULL;
     st_lookup(heap_recorder->object_records, active_recording->obj_id, (st_data_t *) &existing_record);
-    if (existing_record == NULL) rb_raise(rb_eRuntimeError, "Unexpected NULL when reading existing record");
+    if (existing_record == NULL) rb_raise(datadog_profiling_error_class, "Unexpected NULL when reading existing record");
 
     VALUE existing_inspect = object_record_inspect(heap_recorder, existing_record);
     VALUE new_inspect = object_record_inspect(heap_recorder, active_recording);
-    rb_raise(rb_eRuntimeError, "Object ids are supposed to be unique. We got 2 allocation recordings with "
+    rb_raise(datadog_profiling_error_class, "Object ids are supposed to be unique. We got 2 allocation recordings with "
       "the same id. previous={%"PRIsVALUE"} new={%"PRIsVALUE"}", existing_inspect, new_inspect);
   }
 }
@@ -781,7 +781,7 @@ static void cleanup_heap_record_if_unused(heap_recorder *heap_recorder, heap_rec
   }
 
   if (!st_delete(heap_recorder->heap_records, (st_data_t*) &heap_record, NULL)) {
-    rb_raise(rb_eRuntimeError, "Attempted to cleanup an untracked heap_record");
+    rb_raise(datadog_profiling_error_class, "Attempted to cleanup an untracked heap_record");
   };
   heap_record_free(heap_recorder, heap_record);
 }
@@ -791,14 +791,14 @@ static void on_committed_object_record_cleanup(heap_recorder *heap_recorder, obj
   // (See PROF-10656 Datadog-internal for details). Just in case, I've sprinkled a bunch of NULL tests in this function for now.
   // Once we figure out the issue we can get rid of them again.
 
-  if (heap_recorder == NULL) rb_raise(rb_eRuntimeError, "heap_recorder was NULL in on_committed_object_record_cleanup");
-  if (heap_recorder->heap_records == NULL) rb_raise(rb_eRuntimeError, "heap_recorder->heap_records was NULL in on_committed_object_record_cleanup");
-  if (record == NULL) rb_raise(rb_eRuntimeError, "record was NULL in on_committed_object_record_cleanup");
+  if (heap_recorder == NULL) rb_raise(datadog_profiling_error_class, "heap_recorder was NULL in on_committed_object_record_cleanup");
+  if (heap_recorder->heap_records == NULL) rb_raise(datadog_profiling_error_class, "heap_recorder->heap_records was NULL in on_committed_object_record_cleanup");
+  if (record == NULL) rb_raise(datadog_profiling_error_class, "record was NULL in on_committed_object_record_cleanup");
 
   // Starting with the associated heap record. There will now be one less tracked object pointing to it
   heap_record *heap_record = record->heap_record;
 
-  if (heap_record == NULL) rb_raise(rb_eRuntimeError, "heap_record was NULL in on_committed_object_record_cleanup");
+  if (heap_record == NULL) rb_raise(datadog_profiling_error_class, "heap_record was NULL in on_committed_object_record_cleanup");
 
   heap_record->num_tracked_objects--;
 
@@ -862,7 +862,7 @@ heap_record* heap_record_new(heap_recorder *recorder, ddog_prof_Slice_Location l
   uint16_t frames_len = locations.len;
   if (frames_len > MAX_FRAMES_LIMIT) {
     // This is not expected as MAX_FRAMES_LIMIT is shared with the stacktrace construction mechanism
-    rb_raise(rb_eRuntimeError, "Found stack with more than %d frames (%d)", MAX_FRAMES_LIMIT, frames_len);
+    rb_raise(datadog_profiling_error_class, "Found stack with more than %d frames (%d)", MAX_FRAMES_LIMIT, frames_len);
   }
   heap_record *stack = calloc(1, sizeof(heap_record) + frames_len * sizeof(heap_frame)); // See "note on calloc vs ruby_xcalloc use" above
   stack->num_tracked_objects = 0;
@@ -933,21 +933,21 @@ static void unintern_or_raise(heap_recorder *recorder, ddog_prof_ManagedStringId
 
   ddog_prof_MaybeError result = ddog_prof_ManagedStringStorage_unintern(recorder->string_storage, id);
   if (result.tag == DDOG_PROF_OPTION_ERROR_SOME_ERROR) {
-    rb_raise(rb_eRuntimeError, "Failed to unintern id: %"PRIsVALUE, get_error_details_and_drop(&result.some));
+    rb_raise(datadog_profiling_error_class, "Failed to unintern id: %"PRIsVALUE, get_error_details_and_drop(&result.some));
   }
 }
 
 static void unintern_all_or_raise(heap_recorder *recorder, ddog_prof_Slice_ManagedStringId ids) {
   ddog_prof_MaybeError result = ddog_prof_ManagedStringStorage_unintern_all(recorder->string_storage, ids);
   if (result.tag == DDOG_PROF_OPTION_ERROR_SOME_ERROR) {
-    rb_raise(rb_eRuntimeError, "Failed to unintern_all: %"PRIsVALUE, get_error_details_and_drop(&result.some));
+    rb_raise(datadog_profiling_error_class, "Failed to unintern_all: %"PRIsVALUE, get_error_details_and_drop(&result.some));
   }
 }
 
 static VALUE get_ruby_string_or_raise(heap_recorder *recorder, ddog_prof_ManagedStringId id) {
   ddog_StringWrapperResult get_string_result = ddog_prof_ManagedStringStorage_get_string(recorder->string_storage, id);
   if (get_string_result.tag == DDOG_STRING_WRAPPER_RESULT_ERR) {
-    rb_raise(rb_eRuntimeError, "Failed to get string: %"PRIsVALUE, get_error_details_and_drop(&get_string_result.err));
+    rb_raise(datadog_profiling_error_class, "Failed to get string: %"PRIsVALUE, get_error_details_and_drop(&get_string_result.err));
   }
   VALUE ruby_string = ruby_string_from_vec_u8(get_string_result.ok.message);
   ddog_StringWrapper_drop((ddog_StringWrapper *) &get_string_result.ok);

ext/datadog_profiling_native_extension/profiling.c

@@ -55,6 +55,11 @@ void DDTRACE_EXPORT Init_datadog_profiling_native_extension(void) {
   rb_define_singleton_method(native_extension_module, "native_working?", native_working_p, 0);
   rb_funcall(native_extension_module, rb_intern("private_class_method"), 1, ID2SYM(rb_intern("native_working?")));
 
+  // Initialize the ProfilingError exception class reference
+  // This exception class should be defined in Ruby code (lib/datadog/profiling.rb)
+  datadog_profiling_error_class = rb_const_get(profiling_module, rb_intern("ProfilingError"));
+  rb_global_variable(&datadog_profiling_error_class);
+
   ruby_helpers_init();
   collectors_cpu_and_wall_time_worker_init(profiling_module);
   collectors_discrete_dynamic_sampler_init(profiling_module);
@@ -115,7 +120,7 @@ static VALUE _native_grab_gvl_and_raise(DDTRACE_UNUSED VALUE _self, VALUE except
     grab_gvl_and_raise(args.exception_class, "%s", args.test_message);
   }
 
-  rb_raise(rb_eRuntimeError, "Failed to raise exception in _native_grab_gvl_and_raise; this should never happen");
+  rb_raise(datadog_profiling_error_class, "Failed to raise exception in _native_grab_gvl_and_raise; this should never happen");
 }
 
 static void *trigger_grab_gvl_and_raise(void *trigger_args) {
@@ -151,7 +156,7 @@ static VALUE _native_grab_gvl_and_raise_syserr(DDTRACE_UNUSED VALUE _self, VALUE
     grab_gvl_and_raise_syserr(args.syserr_errno, "%s", args.test_message);
   }
 
-  rb_raise(rb_eRuntimeError, "Failed to raise exception in _native_grab_gvl_and_raise_syserr; this should never happen");
+  rb_raise(datadog_profiling_error_class, "Failed to raise exception in _native_grab_gvl_and_raise_syserr; this should never happen");
 }
 
 static void *trigger_grab_gvl_and_raise_syserr(void *trigger_args) {
@@ -246,7 +251,7 @@ static VALUE _native_trigger_holding_the_gvl_signal_handler_on(DDTRACE_UNUSED VA
 
   replace_sigprof_signal_handler_with_empty_handler(holding_the_gvl_signal_handler);
 
-  if (holding_the_gvl_signal_handler_result[0] == Qfalse) rb_raise(rb_eRuntimeError, "Could not signal background_thread");
+  if (holding_the_gvl_signal_handler_result[0] == Qfalse) rb_raise(datadog_profiling_error_class, "Could not signal background_thread");
 
   VALUE result = rb_hash_new();
   rb_hash_aset(result, ID2SYM(rb_intern("ruby_thread_has_gvl_p")), holding_the_gvl_signal_handler_result[1]);

ext/datadog_profiling_native_extension/ruby_helpers.c

@@ -12,6 +12,10 @@ static ID _id2ref_id = Qnil;
 static ID inspect_id = Qnil;
 static ID to_s_id = Qnil;
 
+// Global reference to Datadog::Profiling::ProfilingError exception class
+// Initialized in profiling.c during extension initialization
+VALUE datadog_profiling_error_class = Qnil;
+
 void ruby_helpers_init(void) {
   rb_global_variable(&module_object_space);
 
@@ -44,7 +48,7 @@ void grab_gvl_and_raise(VALUE exception_class, const char *format_string, ...) {
 
   if (is_current_thread_holding_the_gvl()) {
     rb_raise(
-      rb_eRuntimeError,
+      datadog_profiling_error_class,
       "grab_gvl_and_raise called by thread holding the global VM lock. exception_message: '%s'",
       args.exception_message
     );
@@ -76,7 +80,7 @@ void grab_gvl_and_raise_syserr(int syserr_errno, const char *format_string, ...)
 
   if (is_current_thread_holding_the_gvl()) {
     rb_raise(
-      rb_eRuntimeError,
+      datadog_profiling_error_class,
       "grab_gvl_and_raise_syserr called by thread holding the global VM lock. syserr_errno: %d, exception_message: '%s'",
       syserr_errno,
       args.exception_message