Rethinkdb: Add an `error` optarg to `coerceTo`

[encryptio]

Someone on Slack brought up r.expr("foo").coerceTo("number").gt(2) and wanted to handle the case where the value could not be coereced to a number. r.default doesn't work in this case. The only solution that came up was using r.branch(r.match(...), ...coerceTo('number'), null).gt(2), which is ugly and error prone.

I'd like for an optarg to be added to coerceTo that allows you to customize its error handling, regardless of source and destination types. Specifically:

• r.expr("str").coerceTo("number") raises a coercion error (as it does now)
• r.expr("str").coerceTo("number", error="raise") raises the same coercion error (in other words, "raise" is the default)
• r.expr("str").coerceTo("number", error=r.literal(anyValue)) returns anyValue
• r.expr("str").coerceTo("number", error=null) is a malformed query (and raises an error as such.) The same error is raised with other values of the error optarg other than the string "raise" and calls to r.literal.
• All other type pairs of coercions follow the same rules (failed coercions raise errors in the same situations, setting error=r.literal(...) instead gives you a default value.)

I chose to require r.literal for a default replacement value and all unknown values to be errors because then we can't define new special values without breaking backwards compatibility (and you wouldn't be able to default to "raise", which seems like an unnecessary restriction.)

Allowing the special case of null instead of r.literal(null) is tempting as it's a very useful default value, but for the sake of not misleading users, I think less special cases is better. (I could probably be convinced otherwise for the specific case of null, though.)

This is an extension to the current definition of r.literal which only occurs inside write operations, but I think it's appropriate here (and r.literal has no useful semantics on its own anyway.)

In the future, error= could be extended with other strings for common behavior (not that I can think of any right now), or a ReQL function for arbitrary handling.

Related issues:

• #1993 mentions an alternative to the specific case of string-to-number, taking the ES3 parseInt semantics of using base 36. This proposal is more general, and I think this proposal is also more useful in practice for the string-to-number use case.
• #2269 has some discussion on exception handling in the middle of ReQL queries, which is an alternate way to solve this problem (e.g. catch a CoercionError.)

[meenie]

@encryptio Thanks for writing this up! I appreciate you looking into this so thoroughly :).

[marshall007]

@encryptio instead of having an optarg, what do you think about having a generic term (maybe .catch()) for handling exceptions server-side?

.catch() // catch everything, return `null`
.catch(<type>) // catch errors of `<type>`, return `null`
.catch(<value>) // catch everything, return `<value>`
.catch(<type>, <value>) // catch errors of `<type>`, return `<value>`

Example usage:

let expr = r.expr("str").coerceTo("number");

// the following would all be equivalent in this case:
expr.catch(r.literal(0))
expr.catch('CoercionError', r.literal(0))
expr.catch('ReqlCoercionError', r.literal(0))
expr.catch(r.Error.ReqlCoercionError, r.literal(0))

With this in place, .default() just becomes a special case of .catch().

r.row('foo').default('bar')
r.row('foo').catch(r.Error.ReqlNonExistenceError, r.literal('bar'))

[encryptio]

This is actually forcing me to rethink my thoughts on exceptions (no pun intended.) My gut feeling is "ew pls no", but digging into it, none of my objections apply to ReQL. (The usual ones I have are related to poor documentation of what exceptions can be thrown in what situations, the difficulty of debugging a forgotten exception at one layer that was noticed at a different layer dozens of files and hundreds of stack entries away, bad use of exceptions (like for non-exceptional situations), etc; all of those objections are irrelevant to ReQL.)

The more I think about it, the less I think catch is a bad idea for ReQL in particular. I have some objections about the particular syntax you mention for it (overloaded one-arg catch is a bit scary if you allow string error names, the use of r.literal doesn't seem appropriate, and in the general case of catch I think a function handler should be the primary form with perhaps some shortcuts), but the overall idea of catch in ReQL seems okay after giving it some thought.

[marshall007]

@encryptio all good points. I really like the semantics of .map(), accepting either a constant or function, and that tends to be how you expect most ReQL terms to behave. It would be nice if we could preserve that here. The use of r.literal in my examples was to disambiguate the single-arg catch overloads. In hind-sight that's probably more trouble than it's worth. So I agree that function handlers should be the preferred way of doing things.

[mlucy]

+1 for implementing a generic rescue or catch instead of adding an optarg to every term that produces an error we might want to replace with a value.