From a2a3a0aec21af3440fa2ff3399610c1372c38908 Mon Sep 17 00:00:00 2001 From: Gord Pearson Date: Wed, 10 Jun 2026 07:48:39 -0400 Subject: [PATCH] Make fault: a first-class argument to raise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `fault:` keyword argument to `Dev::Error#initialize` and `CLI::Kit.raise`, allowing callers to attribute exceptions to a fault category at the raise site: raise Dev::IncorrectUsage.new("bad input", fault: Fault::USER) CLI::Kit.raise(SomeError, "failed", fault: Fault::SYSTEM) The `fault:` kwarg on Dev::Error is optional at runtime (defaults to nil, falling back to the subclass's `def fault` default) so that raising can never itself fail — better to raise with a default fault than to raise an ArgumentError while trying to raise the real error. The RequireExplicitRaise cop is updated to enforce fault attribution: - `raise Klass, "msg"` is now banned (Sorbet's Kernel#raise intrinsic cannot be extended with fault:). The cop suggests using `raise MyError.new(..., fault: ...)` for Dev::Error subclasses, or `CLI::Kit.raise ..., fault: ...` for non-Dev::Error subclasses. - `raise Klass.new("msg")` without fault: is flagged. - `CLI::Kit.raise(Klass, "msg")` without fault: is flagged. - `raise RuntimeError.new(...)` is flagged (use a specific class). - `raise Klass.new(fault: ...)` without a message is flagged. The cop is rewritten to use RuboCop node patterns throughout. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/ruby.yml | 2 +- .rubocop.yml | 1 + cli-kit.gemspec | 2 +- lib/cli/kit.rb | 5 +- lib/cli/kit/core_ext.rb | 118 ++++++++++++++++++++++++++++++++++- lib/cli/kit/error_handler.rb | 24 ++++++- 6 files changed, 142 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 1df6f23..62de4c0 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -30,7 +30,7 @@ jobs: strategy: matrix: os: [macos-latest, ubuntu-latest] - ruby-version: ['3.1', '3.2', '3.3', '3.4'] + ruby-version: ['3.2', '3.3', '3.4'] runs-on: ${{ matrix.os }} env: diff --git a/.rubocop.yml b/.rubocop.yml index 9c9cdfe..201f1ac 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,7 @@ inherit_from: plugins: rubocop-sorbet AllCops: + TargetRubyVersion: 3.2 Exclude: - gen/template/**/* - vendor/**/* diff --git a/cli-kit.gemspec b/cli-kit.gemspec index e0b3cb5..9bfb9f8 100644 --- a/cli-kit.gemspec +++ b/cli-kit.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency('cli-ui', '~> 2.4') - spec.required_ruby_version = '>= 3.0' + spec.required_ruby_version = '>= 3.2' spec.add_development_dependency('bundler', '~> 2.1') spec.add_development_dependency('minitest', '~> 5.0') diff --git a/lib/cli/kit.rb b/lib/cli/kit.rb index 920d48f..bcbde36 100644 --- a/lib/cli/kit.rb +++ b/lib/cli/kit.rb @@ -109,7 +109,7 @@ class << self # `depth` is used to trim leading elements of the backtrace. If you wrap # this method in your own wrapper, you'll want to pass `depth: 2`, for # example. - #: (?(Class | String | Exception) exception, ?untyped string, ?Array[String]? array, ?cause: Exception?, ?bug: bool?, ?silent: bool?, ?depth: Integer) -> bot + #: (?(Class | String | Exception) exception, ?untyped string, ?Array[String]? array, ?cause: Exception?, ?bug: bool?, ?silent: bool?, ?fault: Exception::Fault?, ?depth: Integer) -> bot def raise( # default arguments exception = UNTYPED_NIL, @@ -117,7 +117,7 @@ def raise( array = UNTYPED_NIL, cause: $ERROR_INFO, # new arguments - bug: nil, silent: nil, depth: 1 + bug: nil, silent: nil, fault: nil, depth: 1 ) k = Kernel #: as untyped if array @@ -132,6 +132,7 @@ def raise( rescue Exception => e # rubocop:disable Lint/RescueException e.bug!(bug) unless bug.nil? e.silent!(silent) unless silent.nil? + e.fault!(fault) if fault Kernel.raise(e, cause: cause) end end diff --git a/lib/cli/kit/core_ext.rb b/lib/cli/kit/core_ext.rb index 6133a0b..f463a36 100644 --- a/lib/cli/kit/core_ext.rb +++ b/lib/cli/kit/core_ext.rb @@ -6,11 +6,82 @@ class Exception # don't. I'm not sure why. If you, the reader, want to take some time to # figure it out, go ahead and refactor to that. - #: -> bool - def bug? - true + # Fault represents who is responsible for an error. It is used to attribute + # exceptions to the right owner for observability and triage. + # + # @abstract + # @sealed + class Fault + # @abstract + #: -> String + def to_s = raise NotImplementedError, 'abstract method called' + + # A bug in the tool itself (dev, tec, cli-kit, etc.). + class System < Fault + # @override + #: -> String + def to_s = 'system' + end + SYSTEM = System.new.freeze #: System + + # The default fault for exceptions that have not been explicitly attributed. + # Nothing should reference this constant directly — it exists only as the + # default return value of Exception#fault. Use rubocop to enforce this. + # + # Inherits from System so that `fault.is_a?(System)` and checks like + # `bug?` (which uses `Fault::System ===`) treat unattributed errors as + # system errors by default. + class ImplicitlySystem < System + # @override + #: -> String + def to_s = 'implicitly_system' + end + IMPLICITLY_SYSTEM = ImplicitlySystem.new.freeze #: ImplicitlySystem + + # A problem with the project's configuration (dev.yml, zone.nix, etc.). + class Project < Fault + # @override + #: -> String + def to_s = 'project' + end + PROJECT = Project.new.freeze #: Project + + # User error: incorrect CLI usage, expired tokens, bad arguments, etc. + class User < Fault + # @override + #: -> String + def to_s = 'user' + end + USER = User.new.freeze #: User + + # Not a real failure — e.g. user cancellation, interrupt, or conditions + # wildly out of anyone's control (no disk space). Doesn't require fixing + # or tracking. + class Terroir < Fault + # @override + #: -> String + def to_s = 'terroir' + end + TERROIR = Terroir.new.freeze #: Terroir + end + + #: -> Fault + def fault = Fault::IMPLICITLY_SYSTEM + + #: (Fault fault) -> void + def fault!(fault) + singleton_class.define_method(:fault) { fault } + end + + #: (Fault fault) -> self + def with_fault(fault) + fault!(fault) + self end + #: -> bool + def bug? = (Fault::System === fault) + #: -> bool def silent? false @@ -26,3 +97,44 @@ def silent!(silent = true) singleton_class.define_method(:silent?) { silent } end end + +# -- Custom fault defaults for Ruby stdlib exceptions -------------------------- +# +# Normally, fault attribution belongs at the raise site (via `fault:` kwarg). +# These classes are different: Ruby itself raises them (e.g. the kernel sends +# SIGINT, the OS returns ENOSPC), so there is no raise site we control. We +# override the default fault at the class level instead. +# +# All of these are environmental conditions ("terroir") — nobody's fault, +# nothing to fix, shouldn't be reported as bugs. + +class Interrupt # Ctrl-C / SIGINT + #: -> Exception::Fault + def fault = Exception::Fault::TERROIR +end + +class SignalException # SIGTERM, SIGHUP, etc. + #: -> Exception::Fault + def fault = Exception::Fault::TERROIR +end + +module Errno + class ENOSPC # disk full + #: -> Exception::Fault + def fault = Exception::Fault::TERROIR + end +end + +module Errno + class EPIPE # broken pipe + #: -> Exception::Fault + def fault = Exception::Fault::TERROIR + end +end + +module Errno + class EIO # I/O error (e.g. terminal disconnected) + #: -> Exception::Fault + def fault = Exception::Fault::TERROIR + end +end diff --git a/lib/cli/kit/error_handler.rb b/lib/cli/kit/error_handler.rb index 00f1deb..dfe7a3e 100644 --- a/lib/cli/kit/error_handler.rb +++ b/lib/cli/kit/error_handler.rb @@ -84,7 +84,7 @@ def triage_all_exceptions(&block) rescue Interrupt => e # Ctrl-C # transform message, prevent bugsnag exc = e.exception('Interrupt') - CLI::Kit.raise(exc, bug: false) + CLI::Kit.raise(exc, fault: Exception::Fault::TERROIR) rescue Errno::ENOSPC => e # transform message, prevent bugsnag message = if @tool_name @@ -93,7 +93,7 @@ def triage_all_exceptions(&block) 'Your disk is full - free space is required to operate' end exc = e.exception(message) - CLI::Kit.raise(exc, bug: false) + CLI::Kit.raise(exc, fault: Exception::Fault::TERROIR) end # If SystemExit was raised, e.g. `exit()`, then # return whatever status is attached to the exception @@ -125,9 +125,27 @@ def caused_by_benign_signal?(error) false end + # Walks the cause chain looking for anything that should be reported as a + # bug. A terroir wrapper (e.g. Errno::EPIPE raised by a finalizer) can + # mask a real bug as its cause, so we can't only inspect the outermost + # exception. A non-benign signal (e.g. SIGSEGV) also counts as a bug. + #: (Exception? error) -> bool + def caused_by_bug?(error) + current = error #: Exception? + while current + return true if current.bug? + if current.is_a?(SignalException) && !SIGNALS_THAT_ARENT_BUGS.include?(current.message) + return true + end + + current = current.cause + end + false + end + #: (Exception? error) -> Exception? def exception_for_submission(error) - return if error.nil? || !error.bug? || caused_by_benign_signal?(error) + return if error.nil? || caused_by_benign_signal?(error) || !caused_by_bug?(error) case error when SignalException