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