Exceptions can be hard to catch in test environments

Inizia da lib/temporal/workflow.rb, intorno al rescue di StandardError, e segui il modo in cui Temporal::Testing.local! esegue i workflow. Riproduci l'esempio della keyword mancante, quindi individua e documenta un comportamento dei test locali in cui le eccezioni del workflow emergono senza modificare la normale gestione dei workflow; il lavoro è completato quando il test espone l'eccezione con uno stack trace utile.

Cool library, and thank you for your hard work on it! I'm trying to help improve it through this feedback, and would be willing to open a PR if there's guidance provided on desirable compromises or solutions.

The way that Temporal::Workflow rescues StandardError makes catching exceptions in tests hard.

Take this spec as an example:

$global_var = 0

class HelloWorldWorkflow < Temporal::Workflow
  def execute(arg1, required_key:)
    $global_var += 1

    nil
  end
end

describe HelloWorldWorkflow do
  it "doesn't allow exceptions to surface very easily" do
    Temporal::Testing.local! do
      Temporal.start_workflow(HelloWorldWorkflow, "foo", required_key: "bar")

      expect($global_var).to eq(1)

      Temporal.start_workflow(HelloWorldWorkflow, "foo") # incorrect arguments

      expect($global_var).to eq(2)
    end
  end
end

The result is:

expected: 2
     got: 1

(compared using ==)

Yes, I can see in the logs that an exception is logged, but I feel like in tests raising exceptions should be the rule. Any number of things can break downstream within a workflow, and sometimes (when not directly testing the unit that is the workflow) it's useful to call a thing, and not have to make an assertion that it didn't log an exception -- if that makes sense. Rescuing StandardError is heavy handed in the tests, because allowing those to raise is way more useful in identifying any issues.

To address this I've wrapped the base class with my own, so I can re-raise the exception in a way that Temporal doesn't try to handle. I'm wondering if Temporal::Testing should take this into consideration and not rescue any exceptions when executing the workflow locally in tests.

class ApplicationWorkflow < Temporal::Workflow
  def execute(*args, **kwargs)
    perform(*args, **kwargs)
  rescue => e
    raise(Exception, e.message) if defined?(Temporal::Testing) && Temporal::Testing.local?
    raise(e)
  end
end

class HelloWorldWorkflow < ApplicationWorkflow
  def perform(arg1, required_key:)
    $global_var += 1

    nil
  end
end

And the more useful result now includes the exception and stops the execution of the spec, which is what would be most helpful.

Exception: missing keyword: :required_key

It's still not the best solution though because it now has two rescues and pollutes the stack trace.