Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silent Error Handling in Fluxus::Safe::Caller Reduces Error Visibility #14

Open
ViniMoraes opened this issue Aug 20, 2024 · 4 comments
Open

Comments

@ViniMoraes
Copy link

The Fluxus::Safe::Caller class handles errors by silencing them, which can lead to a lack of visibility into underlying issues. This behavior can make debugging difficult as errors are not logged or raised, potentially leading to unexpected behavior without a clear indication of the cause.

Proposed Solution:

  • Introduce optional logging or error reporting when exceptions are caught in Fluxus::Safe::Caller.
  • Add configuration options to control error handling (e.g., re-raise, log, or silence).

Impact:

Improving error visibility will help developers identify and fix issues more efficiently, leading to more robust and maintainable code.

@Rynaro
Copy link
Owner

Rynaro commented Aug 21, 2024

Fluxus::Safe::Caller is designed to bring safety to the runtime. All errors are self-contained in the Fluxus::Results::Result, and take the common aspect of a Failure.
The Fluxus::Safe is also optant of #on_exception specific chainable method. This method makes the code more explicit about what to do after an expected error.
For example, during a payment flow, you can have a specific error type for gateway failures.

PayWithCreditCard.
  call(credit_card: my_cc_object, payment_gateway_wrapper: my_wrapper)
  on_success { |result| invoke_checkout_success(result) }.
  on_exception(MyGateway::InsuficientFunds) { |result| invoke_checkout_failure(result) }
  on_exception(MyGateway::FraudulentOperation) { |result| invoke_operational_fraud_and_report(result) }.
  on_failure { |result| invoke_unexpected_failure(result) }

Not-mapped errors will always invoke failures, and guarantee the Ruby runtime continuity by taking wrapping the error in a "sandboxed" object. If you do not properly handle the Safe object, silent failures will likely happen in your code.

If need to use the "let it crash" approach I strongly recommend you use the common Fluxus::Caller.

@ViniMoraes
Copy link
Author

Thank you for the detailed explanation. I understand the design philosophy behind Fluxus::Safe::Caller and its focus on safety by encapsulating errors within a Failure result. However, my concern is with the visibility of these errors during runtime. While the exceptions don’t need to interrupt the flow, having at least an optional log would greatly aid in debugging and monitoring. This log could help identify underlying issues without compromising the flow control that Safe::Caller provides.

A suggestion would be to add a configurable logger in Safe::Caller. Something like this:

module Fluxus
  module Safe
    class Caller < Runner
      class << self
        attr_writer :logger

        def logger
          @logger ||= Logger.new($stdout)
        end
      end
      def self.call!(...)
        instance = new
        __call__(instance, ...)
      rescue StandardError => e
        raise e if e.is_a?(ResultTypeNotDefinedError)

        logger&.error("Exception caught in #{name}: #{e.message}")
        instance.Failure(type: :exception, result: { exception: e })
      end
    end
  end
end

this add the possibility to configure a logger in Rails for example:

# config/initializers/fluxus_logger.rb

Fluxus::Safe::Caller.logger = Rails.logger

@Rynaro
Copy link
Owner

Rynaro commented Aug 22, 2024

@ViniMoraes I see your concerns, and I'm open to evaluating a pull request implementing the proposed idea!
I have a few suggestions before you, or anyone else start implementing it.

  • Safe navigators are great programming resources, and Ruby makes it even easier to implement, but we are talking about a library that is responsible for controlling a sort of other code workflows. Avoid creating those dubious implementations. The logger object must need to exist and be valid before calling it!
  • Do not force other devs to use a tool that they do not ask for. If they do not want to use any logging mechanism in Fluxus we must not make an instance of an object that is not needed. This change also solves the safe navigator suggestion.
  • With the logger addition we just need to create a "singleton" configuration. This will save resources, the Fluxus major instance will always serve the logger object without needing to have a Fluxus instance always memory loaded or constantly creating a logger change.
  • The logger should be an entity in Fluxus, create an idiomatic implementation about it. e.g.: Fluxus::Accessories::Logger.

Following those suggestions we are good to have it.

@clsechi
Copy link

clsechi commented Aug 23, 2024

Another proposal based on Sidekiq implementation of death_handlers, is receive a lambda on the Fluxus configuration that will be called every time an exception happens. The logic of what is going to be done when a exception happens will be kept outside of Fluxus, as the user can do more than just logging the message.

# config/initializers/fluxus.rb

Fluxus::Safe::Caller.exception_handler = ->(exception) { ... }
module Fluxus
  module Safe
    class Caller < Runner
      class << self
        attr_writer :logger

        def logger
          @logger ||= Logger.new($stdout)
        end
      end
      def self.call!(...)
        instance = new
        __call__(instance, ...)
      rescue StandardError => e
        raise e if e.is_a?(ResultTypeNotDefinedError)

        exception_handler.call(e) unless exception_handler.nil?
        instance.Failure(type: :exception, result: { exception: e })
      end
    end
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants