SiriusResult should wrap Throwable (issue #8)

[smuir]

There's no particular reason why SiriusResult can only wrap RuntimeException as an error, so generalise it to Throwable (analogous to the Scala Try class).

See https://github.com/Comcast/sirius/issues/8

[comcast-jonm]

There's another backwards-compatibility angle here, which is that old client code looking like this:

SiriusResult result = ...;
if (result.isError()) {
  try {
    result.getValue();
  except (RuntimeException re) {
    /* this catches everything that can be thrown here */
  }
}

May get a subtle surprise if a SiriusResult somehow shows up with a checked exception or an Error.

However, I am +1 on this being part of a revamped 1.2 interface, which I'll get around to adding shortly.

[smuir]

For that to be an issue in practice it would require that somehow existing code managed to get a superclass of RuntimeException wrapped in a SiriusResult. Is that possible? A backwards-compatible solution might be to not change the error(RuntimeException) factory method, but add an alternative used to wrap Throwable

[smuir]

Anyway, I think the 'better' solution, as discussed in email, is a different API that uses Scala native types rather than SiriusResult, at least for future clients.

[clinedome-work]

Hmm, if we did this, wouldn't the method signature of anything in Java that calls getValue() have to change? I imagined that would get inferred through, but I'm not entirely sure.

Whether SiriusResult is the right answer or not, I'd say there's value in avoiding the nested Scala-native types, even if they do most succinctly describe what's happening. They're a total bear to work with in Java.

All for looking at this stuff in more detail for 1.2 though.

[smuir]

Interesting question. I don't think the signature of getValue() is changed by changing the underlying type of the exception field.

[clinedome-work]

Upon testing, it doesn't. :)

[smuir]

Updated to add an exception() method to wrap Throwable, with error() being reverted to the original interface of accepting only a RuntimeException

[smuir]

Just made a minor to change to make it possible to directly retrieve the Throwable associated with a SiriusResult.

[smuir]

The Travis CI build sometimes seems to fail for one of the two versions of Scala compiler, but not the other. Anyone got any ideas why?

[comcast-jonm]

This might need rebasing and resubmitting now that we're on a public TravisCI build.

[smuir]

If we are going to consider a new 1.3 release that has a more standard return type than SiriusResult, a la #26, then this might not be worth bothering with. But it seems like #26 was generally decided against, in which case this could easily be respun if it was thought to be useful. It probably isn't actually useful until we get to the point that return values actually propagate from the RequestHandler (issue #6 )

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}