Optional error information instead of throwing (by default)

[dcousens]

From #42

Type functions should simply always return false on failure rather than returning false OR throwing.

The problem is, without extra information, the error messages would suck, so return false AND add the extra information, could give the best of both.

[treygriffith]

Maybe I'm missing a class of use cases, but it seems to me that type functions should always throw rather than return false. This allows the user to make a determination about whether and how to proceed, with the relevant error message (via a typical try/catch).

Of course, if type checking without throwing is a common enough pattern you could add some sugar that wraps the typical type function in a try catch that returns false. I'm trying to imagine why you would need both an exception-less return AND access to the exception, but if that's another pattern to support you could add error saving to the sugary function. At least it would keep it in one place.

[dcousens]

@treygriffith often you'll have functions like isBase64(string), as is, you can directly use that as a type with typeforce and things will work as expected.
This is great for API's where you want to use typeforce to throw TypeErrors, but otherwise not need to write any wrappers to operate with it.

If you presumed to always throw, we would not support that use case without tedious wrappers.
As my own usecases tend to have a lot of the is* functions, that I then use as types, I prefer the never throw approach.

The ambiguity is what I want to remove, settling on one or the other.

However, if you never throw, you don't get the reason for why the type was non conformant, and instead you are left with a high level reason which may be misleading.

Another problem with throwing is that performance sucks for recursive/deep types, try...catch...try...catch..., and you can't guaranatee your types work safely without a try/catch block.

This PR sticks with the return false narrative, with error information accessible if required.

[treygriffith]

@dcousens that makes sense given the use case you highlighted (and the performance impact).

Might I suggest then keeping the error saving in one place rather than attaching it as an property to the various functions?

I'm imagining something like this:

var lastTypeError

function setLastTypeError(err) {
  lastTypeError = err
}

getLastTypeError() {
  return lastTypeError
}  

function Length (value) {
   if (!type(value)) return false
   if (value.length === length) return true
		  
   setLastTypeError(ERRORS.tfCustomError(name + '(Length: ' + length + ')', name + '(Length: ' + value.length + ')'))
   return false
}

Might not be your style or there might be another reason to do it the way you have it, but feels better to manage the setting and getting of the error information in one place.

[dcousens]

Great point, but how do you support external types?
Expose setLastTypeError publicly?

Additionally, what resets the field? Any type that might set it I suppose?

[treygriffith]

Yeah, I would think exposing setLastTypeError publicly makes the most sense for allowing external types to take advantage.

As for reset, I think it makes the most sense to have next type error override it. I would expect consumers of the API to check the return value of the type function and then check for the lastTypeError - it's existence shouldn't mean anything on its own.

[dcousens]

@treygriffith I suspect that typeforce will have to make the call that resets the error information, as the only viable option, as otherwise users will be too prone to bugs of forgetting to reset etc.

[dcousens]

check the return value of the type function and then check for the lastTypeError - it's existence shouldn't mean anything on its own.

Aye, but what if they didn't call setLastTypeError, but a previous-previous error was there?
Who resets that? Every function that might call setLastTypeError, has to call resetLastTypeError at the start?
(similar to this PR)

[dcousens]

At least some of the benefits for this can be found in #53

[dcousens]

I suppose the approach taken in #53 might be better.
Coupled with exposing an .error object, and if you use typeforce it resets any .error found on any types given.

[treygriffith]

@dcousens #53 looks like you went with "always throw, but add a wrapper to return false". Is that right?

[dcousens]

@treygriffith indeed, until I find the ideal solution for 2.0.0.
The problem is I set a standard of accepting throw to begin with, which means a breaking change to avoid that would be catastrophically annoying for any custom type functions used around the place.