Raise ValueError/NotImplementedError instead of a generic RuntimeError based on DP error code

src/bindings/PyDP/pydp_lib/algorithm_builder.hpp

@@ -11,6 +11,7 @@
 #include "algorithms/numerical-mechanisms.h"
 #include "algorithms/order-statistics.h"
 #include "proto/summary.pb.h"
+#include "status_errors.hpp"
 
 namespace dp = differential_privacy;
 namespace py = pybind11;
@@ -87,7 +88,7 @@ class AlgorithmBuilder {
 
     absl::StatusOr<std::unique_ptr<Algorithm>> obj = builder.Build();
     if (!obj.ok()) {
-      throw std::runtime_error(obj.status().ToString());
+      ThrowFromStatus(obj.status());
     }
 
     return std::move(obj.value());
@@ -176,7 +177,7 @@ class AlgorithmBuilder {
       auto result = pythis.Result(v.begin(), v.end());
 
       if (!result.ok()) {
-        throw std::runtime_error(result.status().ToString());
+        ThrowFromStatus(result.status());
       }
       if constexpr ((should_return_T<T, Algorithm>()))
         return dp::GetValue<T>(result.value());
@@ -190,7 +191,7 @@ class AlgorithmBuilder {
       auto result = pythis.PartialResult();
 
       if (!result.ok()) {
-        throw std::runtime_error(result.status().ToString());
+        ThrowFromStatus(result.status());
       }
 
       if constexpr ((should_return_T<T, Algorithm>()))
@@ -205,7 +206,7 @@ class AlgorithmBuilder {
       auto result = pythis.PartialResult(privacy_budget);
 
       if (!result.ok()) {
-        throw std::runtime_error(result.status().ToString());
+        ThrowFromStatus(result.status());
       }
 
       if constexpr ((should_return_T<T, Algorithm>()))
@@ -220,7 +221,7 @@ class AlgorithmBuilder {
       auto result = pythis.PartialResult(noise_interval_level);
 
       if (!result.ok()) {
-        throw std::runtime_error(result.status().ToString());
+        ThrowFromStatus(result.status());
       }
       if constexpr ((should_return_T<T, Algorithm>()))
         return dp::GetValue<T>(result.value());
@@ -238,7 +239,7 @@ class AlgorithmBuilder {
     pyself.def("merge", [](Algorithm& pythis, const dp::Summary& summary) {
       auto status = pythis.Merge(summary);
       if (!status.ok()) {
-        throw std::runtime_error(status.ToString());
+        ThrowFromStatus(status);
       }
     });
 

src/bindings/PyDP/pydp_lib/status_errors.hpp

@@ -0,0 +1,44 @@
+#ifndef PYDP_LIB_STATUS_ERRORS_H_
+#define PYDP_LIB_STATUS_ERRORS_H_
+
+#include <pybind11/pybind11.h>
+
+#include <string>
+
+#include "absl/status/status.h"
+
+namespace py = pybind11;
+
+namespace differential_privacy {
+namespace python {
+
+// Raises a Python exception for a non-OK absl::Status coming back from the DP
+// library, picking the exception type based on the status code instead of
+// collapsing every failure into a generic RuntimeError.
+//
+// This lets callers distinguish, for example, a bad argument they passed in
+// (ValueError) from a method that simply isn't implemented for a given
+// algorithm (NotImplementedError), the same way they would for any other
+// Python API. Status codes that don't have an obviously better fit (Internal,
+// FailedPrecondition, etc.) keep raising RuntimeError, which was already the
+// behavior for every code before this.
+//
+// `status` is expected to be non-OK; callers should check status.ok() (or
+// statusor.ok()) first.
+inline void ThrowFromStatus(const absl::Status& status) {
+  const std::string message = status.ToString();
+  switch (status.code()) {
+    case absl::StatusCode::kInvalidArgument:
+      throw py::value_error(message);
+    case absl::StatusCode::kUnimplemented:
+      PyErr_SetString(PyExc_NotImplementedError, message.c_str());
+      throw py::error_already_set();
+    default:
+      throw std::runtime_error(message);
+  }
+}
+
+}  // namespace python
+}  // namespace differential_privacy
+
+#endif  // PYDP_LIB_STATUS_ERRORS_H_

tests/algorithms/test_bounded_mean.py

@@ -86,10 +86,14 @@ def test_serialize_merge():
 
 
 def test_result_crash():
+    # Calling result()/partial_result() a second time trips the DP library's
+    # "InvalidArgument" check (an algorithm can only produce results once for
+    # a given epsilon/delta budget), which the bindings now surface as a
+    # ValueError instead of a generic RuntimeError. See #413.
     bm1 = BoundedMean(1, 0, 1, 10)
     bm1.add_entries([1 for i in range(100)])
     bm1.result()
-    with pytest.raises(RuntimeError):
+    with pytest.raises(ValueError):
         bm1.result()
 
 

tests/algorithms/test_error_types.py

@@ -0,0 +1,50 @@
+# third party
+import pytest
+
+# pydp absolute
+import pydp._pydp as dp
+import pydp.algorithms.laplacian as py_algos
+
+# Regression tests for #413: the bindings used to collapse every non-OK
+# absl::Status coming back from the DP library into a plain RuntimeError,
+# no matter what actually went wrong. That made it impossible to tell "you
+# passed a bad argument" apart from "something failed internally" without
+# parsing the exception message. These tests pin down the exception types
+# for the status codes that are easy to trigger deterministically from
+# Python: InvalidArgument (now ValueError) and Internal (still RuntimeError,
+# since there's no better built-in fit).
+
+
+def test_double_result_raises_value_error_not_runtime_error():
+    # The DP library refuses to hand out a result twice for the same
+    # epsilon/delta budget and returns an InvalidArgument status for the
+    # second call. That should surface as a ValueError.
+    count = py_algos.Count(epsilon=1.0)
+    count.add_entries([1] * 10)
+
+    count.result()
+    with pytest.raises(ValueError):
+        count.result()
+
+
+def test_double_result_error_is_not_a_bare_runtime_error():
+    # Guard against a ValueError subclassing RuntimeError one day and this
+    # test passing for the wrong reason.
+    count = py_algos.Count(epsilon=1.0)
+    count.add_entries([1] * 10)
+    count.result()
+
+    with pytest.raises(ValueError) as exc_info:
+        count.result()
+    assert not isinstance(exc_info.value, RuntimeError)
+
+
+def test_merge_empty_summary_still_raises_runtime_error():
+    # Merging an empty Summary trips an Internal status in the DP library
+    # (there's no count data to merge). Internal errors don't have an
+    # obviously-better built-in Python exception, so they should keep
+    # raising RuntimeError, same as before this change.
+    count = py_algos.Count(epsilon=1.0)
+
+    with pytest.raises(RuntimeError):
+        count.merge(dp.Summary())