Misleading `ServiceActor::ArgumentError` message under specific circumstances

[viralpraxis]

Given the following actor

class TestActor < Actor
  input :value, type: Integer
  output :value_result, type: Integer, allow_nil: true
end

Evaluating TestActor.call(value: 1) leads to

ServiceActor::ArgumentError:
  The "value_result" input on "TestActor" is missing

(note the "input" while value_result is defined as output)

It does not happen if you define output without allow_nil: true

[sunny]

Oh indeed. Right now the options only make sense to inputs as they are checked on call.

I’d be tempted to raise an error on any option passed to output as they are not supported.

Or we could make sure they actually work, but that would mean doing the check on the value_result= method call instead, I guess?

[viralpraxis]

Or we could make sure they actually work, but that would mean doing the check on the value_result= method call instead, I guess?

I'd prefer to make it working, it seems to be quite useful

[sunny]

Happy to accept a PR to make type: work for outputs 🙏🏻

Actually instead of on the #value_result= method call it would make more sense to do that at the end of the #call, when exiting the actor.

[viralpraxis]

Hm, It seems like I misunderstood you.

The following actor

# frozen_string_literal: true

class AddGreetingWithDefault < Actor
  input :name, default: "world", type: String
  output :greeting, type: String

  def call
    self.greeting = 1
  end
end

works quite good, it throws

 ServiceActor::ArgumentError:
   The "greeting" output on "AddGreetingWithDefault" must be of type "String" but was "Integer"

[viralpraxis]

It seels like the probelm is hardcoded input

https://github.com/sunny/actor/blob/bf261d7fa310ae300ad78b4c7e354f7da3335681/lib/service_actor/checks/default_check.rb#L52

which should be replaced with @origin

[sunny]

Yes! My bad, I’ve misunderstood the issue. I forgot that checks actually worked on outputs, I just never used them 🙈

I think the DefaultCheck doesn’t make much sense on outputs and should be probably be skipped in that case.

[viralpraxis]

Just to make sure I got you correctly, current behavior of

class WithUnsetOutput < Actor
  output :value, type: String, allow_nil: true
end

is

ServiceActor::ArgumentError: The "value" input on "WithUnsetOutput" is missing

And if we skep default_check on outputs we'd expect it to complete successfuly with value == nil, right?

[sunny]

Yeah, I think that would make sense.