enhance: add error-handling ratchet gate (abort/throw baseline burn-down)

.github/workflows/error-handling-ratchet.yml

@@ -0,0 +1,61 @@
+name: error-handling-ratchet
+
+# No `paths` filter on purpose: the script runs in seconds, and a path-filtered
+# workflow marked as a required check would leave non-cpp PRs permanently
+# pending (the workflow never runs, so the required status never reports).
+on:
+  push:
+  pull_request:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  check:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: fetch base baseline
+        if: github.event_name == 'pull_request'
+        # The base branch's baseline closes the self-reference loophole:
+        # regenerating a raised baseline inside the PR cannot pass, because
+        # category totals are compared against what the base branch records.
+        #
+        # Assumptions and trust boundary (explicit):
+        # * Layer 2 trusts the baseline COMMITTED on the base rather than
+        #   recomputing from the base tree; the `push` trigger keeps that file
+        #   in sync with the base code under normal operation (drift shows up
+        #   as a red push build on the base branch).
+        # * Like any in-repo lint gate, the checker runs from the PR checkout;
+        #   the defense target is accidental regression, not a malicious PR
+        #   (edits to the gate itself are visible in the PR diff).
+        # * A fetch failure degrades to layer 1 only, with a visible warning:
+        #   blocking unrelated PRs on an infra flake costs more than briefly
+        #   losing the base-totals layer.
+        run: |
+          BASELINE_PATH="cpp/scripts/error_handling_baseline.tsv"
+          # Anchor to the PR's recorded base commit, not the moving branch tip,
+          # so both sides of the comparison agree even if the base just moved.
+          BASE_SHA="${{ github.event.pull_request.base.sha }}"
+          if ! git fetch --depth=1 origin "$BASE_SHA"; then
+            echo "::warning::error-handling-ratchet: could not fetch base commit ${BASE_SHA}; base-totals layer skipped (exact-match layer still enforced)"
+            exit 0
+          fi
+          if git cat-file -e "${BASE_SHA}:${BASELINE_PATH}" 2>/dev/null; then
+            git show "${BASE_SHA}:${BASELINE_PATH}" > /tmp/base_baseline.tsv
+          else
+            # Bootstrap only: the base predates the ratchet. Any other reason
+            # for a missing file (e.g. the baseline was renamed but this
+            # workflow still points at the old path) must NOT silently disable
+            # the layer -- hence the existence check above instead of a blanket
+            # `|| true` on `git show`.
+            echo "::notice::error-handling-ratchet: no baseline at base commit (bootstrap); base-totals layer skipped"
+          fi
+
+      - name: error-handling ratchet
+        run: bash cpp/scripts/error_handling_ratchet.sh check /tmp/base_baseline.tsv

cpp/Makefile

@@ -1,6 +1,7 @@
 .PHONY: build clean test test-all test-cloud-storage \
 	python-lib java-lib package \
 	fix-format check-format fix-tidy check-tidy \
+	check-error-ratchet update-error-ratchet \
 	aws gcp azure aliyun tencent huawei
 
 TEST_THREADS ?= 4
@@ -132,6 +133,12 @@ check-format:
 		find $$dir -type f \( ! -name "*.pb.h" \) \( -iname "*.c" -o -iname "*.h" -o -iname "*.cpp" \) -exec clang-format --dry-run --Werror {} +; \
 	done
 
+check-error-ratchet:
+	@bash scripts/error_handling_ratchet.sh check
+
+update-error-ratchet:
+	@bash scripts/error_handling_ratchet.sh update
+
 check-tidy:
 	python3 ./scripts/run-clang-tidy.py -p build/${build_type}
 

cpp/scripts/error_handling_baseline.tsv

@@ -0,0 +1,13 @@
+throw	cpp/src/common/metadata.cpp	4
+throw	cpp/src/ffi/ffi_fiu_c.cpp	1
+throw	cpp/src/ffi/v2_column_groups_builder.cpp	5
+throw	cpp/src/format/bridge/rust/src/iceberg_bridge.cpp	2
+throw	cpp/src/format/bridge/rust/src/lance_bridge.cpp	21
+throw	cpp/src/format/iceberg/iceberg_common.cpp	2
+throw	cpp/src/format/lance/lance_common.cpp	2
+throw	cpp/src/format/vortex/vortex_translater.cpp	6
+throw	cpp/src/manifest.cpp	1
+throw	cpp/src/packed/chunk_manager.cpp	2
+throw	cpp/src/packed/column_group.cpp	1
+throw	cpp/src/packed/reader.cpp	1
+throw	cpp/src/properties.cpp	1

cpp/scripts/error_handling_ratchet.sh

@@ -0,0 +1,168 @@
+#!/usr/bin/env bash
+# Copyright 2025 Zilliz
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Error-handling ratchet.
+#
+# Library code must report failures as arrow::Status / arrow::Result (with an
+# ExtendStatusDetail tag where classification matters), not by aborting the
+# process or throwing exceptions across the library boundary. Existing
+# violations are grandfathered in a checked-in baseline and burned down over
+# time; this script keeps the count from ever growing back.
+#
+# Category counted (per line, per file):
+#   throw : any `throw` statement (raw std exceptions, bridge exception types,
+#           rethrows). Ring-1 discipline: the library must not leak exceptions,
+#           so in library code ANY throw is a violation -- textual counting IS
+#           the semantic judgment for this category. Comments are stripped
+#           before counting (gcc -fpreprocessed -E removes comments without
+#           expanding includes), so commented mentions do not count; string
+#           literals containing `throw` would still count (none exist today).
+#           A clang-query `cxxThrowExpr()` match would be exact but needs a
+#           compile_commands.json (a configured toolchain job); this gate
+#           stays toolchain-free, and the baseline flow can be reused if that
+#           upgrade happens.
+#
+# Deliberately NOT counted: ValueOrDie/ValueUnsafe (abort-capable accessors).
+# A text-level gate cannot tell the guarded FFI idiom (`.ok()` check + macro
+# return + ValueOrDie -- the house style in cpp/src/ffi/) from an unguarded
+# abort path, so counting them would only nag legitimate additions without
+# distinguishing dangerous ones (maintainer feedback on #596). Unguarded
+# aborts remain a review concern; a future clang-query-based check could
+# reintroduce them with real semantic discrimination.
+#
+# Scope: git-tracked *.cpp / *.cc / *.h / *.hpp under cpp/src and cpp/include.
+# cpp/test is out of scope; vendored or generated trees are excluded simply by
+# not being git-tracked.
+#
+# Known counting limits (kept simple on purpose; the gate is a ratchet, not a
+# semantic linter):
+#   * matches inside comments/strings are counted too, so a file can mask a
+#     new real site by deleting a commented one (and unrelated comment edits
+#     require a baseline regen);
+#   * grep -c counts matching LINES, not call sites: two sites on one line
+#     count once, and a site added to an already-matching line does not trip
+#     the gate.
+# Migrating to clang-query later can reuse the same baseline flow.
+#
+# Enforcement is two-layered:
+#   1. The committed baseline must EXACTLY match the current tree
+#      ("regenerate on any change" keeps the file honest):
+#        - count went UP   -> fix the new code (return Status, don't abort/throw)
+#        - count went DOWN -> record the burn-down: run
+#            cpp/scripts/error_handling_ratchet.sh update
+#          and commit the regenerated baseline in the same PR.
+#   2. In CI, per-category TOTALS are additionally compared against the PR's
+#      BASE branch baseline and must not increase. This closes the
+#      self-reference loophole: regenerating a *raised* baseline inside the
+#      same PR keeps layer 1 green but fails layer 2 — the ratchet direction
+#      is machine-enforced, not review-enforced. (Totals rather than per-file,
+#      so moving grandfathered code between files stays neutral.)
+#
+# Usage:
+#   error_handling_ratchet.sh check [base_baseline.tsv]
+#   error_handling_ratchet.sh update
+
+set -euo pipefail
+
+cd "$(dirname "$0")/../.."  # repo root
+BASELINE="cpp/scripts/error_handling_baseline.tsv"
+MODE="${1:-check}"
+BASE_BASELINE="${2:-}"
+
+collect() {
+  git -c core.quotePath=false ls-files -z -- 'cpp/src' 'cpp/include' \
+    | while IFS= read -r -d '' f; do
+        case "$f" in
+          *.cpp | *.cc | *.h | *.hpp) ;;
+          *) continue ;;
+        esac
+        throw_n=$(gcc -fpreprocessed -dD -E -P "$f" 2>/dev/null | grep -cE '\bthrow\b' || true)
+        if [ "$throw_n" -gt 0 ]; then printf 'throw\t%s\t%s\n' "$f" "$throw_n"; fi
+      done | LC_ALL=C sort
+}
+
+summarize() {
+  awk -F'\t' '{sum[$1]+=$3} END {for (c in sum) printf "  %s: %d\n", c, sum[c]}' "$1" | LC_ALL=C sort
+}
+
+# Compare per-category totals of $2 (current) against $1 (base baseline);
+# fail if any category's total increased.
+check_totals_against_base() {
+  local base_file="$1" current_file="$2"
+  local violations
+  violations=$(awk -F'\t' '
+    FNR == NR { base[$1] += $3; next }
+    { cur[$1] += $3 }
+    END {
+      for (c in cur) if (cur[c] > base[c] + 0)
+        printf "  %s: %d (base) -> %d (this PR)\n", c, base[c] + 0, cur[c]
+    }' "$base_file" "$current_file")
+  if [ -n "$violations" ]; then
+    echo "error-handling ratchet: category totals INCREASED versus the base branch:" >&2
+    echo "$violations" >&2
+    echo >&2
+    echo "New abort/throw sites were added to library code. Raising the committed" >&2
+    echo "baseline cannot pass this check: totals are compared against the BASE" >&2
+    echo "branch's baseline. Return arrow::Status/arrow::Result instead; tag with" >&2
+    echo "ExtendStatusDetail where classification matters." >&2
+    return 1
+  fi
+  return 0
+}
+
+case "$MODE" in
+  update)
+    collect > "$BASELINE"
+    echo "Baseline regenerated: $BASELINE"
+    summarize "$BASELINE"
+    ;;
+  check)
+    if [ ! -f "$BASELINE" ]; then
+      echo "error: baseline $BASELINE not found; run '$0 update' and commit it" >&2
+      exit 1
+    fi
+    current="$(mktemp)"
+    trap 'rm -f "$current"' EXIT
+    collect > "$current"
+
+    # Layer 2 (CI): the ratchet direction, enforced against the base branch.
+    if [ -n "$BASE_BASELINE" ] && [ -s "$BASE_BASELINE" ]; then
+      check_totals_against_base "$BASE_BASELINE" "$current"
+    fi
+
+    # Layer 1: the committed baseline must match the tree exactly.
+    if diff -u "$BASELINE" "$current" > /dev/null; then
+      echo "error-handling ratchet: OK"
+      summarize "$BASELINE"
+    else
+      echo "error-handling ratchet: counts diverged from $BASELINE" >&2
+      echo >&2
+      diff -u "$BASELINE" "$current" >&2 || true
+      echo >&2
+      echo "If a count went UP: new abort/throw sites were added to library code." >&2
+      echo "  Return arrow::Status/arrow::Result instead (tag with ExtendStatusDetail" >&2
+      echo "  where classification matters); raising the baseline will not pass CI," >&2
+      echo "  which also compares category totals against the base branch." >&2
+      echo "If a count went DOWN: thanks for the burn-down - record it by running" >&2
+      echo "  cpp/scripts/error_handling_ratchet.sh update" >&2
+      echo "  and committing the regenerated baseline in this PR." >&2
+      exit 1
+    fi
+    ;;
+  *)
+    echo "usage: $0 [check [base_baseline.tsv]|update]" >&2
+    exit 2
+    ;;
+esac