zenparsing / es-typed-catch

Catch anything you like!
10 stars 0 forks source link

Predicates #2

Open benjamingr opened 8 years ago

benjamingr commented 8 years ago

In my experience with Bluebird (which provides a typed catch) there is an issue with only matching errors by type caused mostly by the fact that most custom errors raised by environments (like Node) do not have meaningful types - but instead patch things like a .code property.

@petkaantonov created a small (but useful) library to work around this here: https://github.com/petkaantonov/core-error-predicates and bluebird also accepts functions as error guards.

Another motivation is that C# 6 added new catch-by-predicate syntax.

`catch(Exception e) when (e.code == 500)` 

I wonder if it wouldn't be helpful to support functions:

catch(e : e.code === 5)
petkaantonov commented 8 years ago

Why not just take a function? catch(e : e => e.code == 500)

benjamingr commented 8 years ago

Yes, in second thought e : e.code === 5 is problematic since you wouldn't be able to reuse logic as nicely.

zenparsing commented 8 years ago

I think in C# it looks like:

try {}
catch (Exception ex) if (ex.message == "foo") {}

I think predicates would be a great extension to this proposal in much the same form:

try {}
catch (error : HttpError) if (error.code === 503) {}

@petkaantonov We need the catch head to contain a binding identifier or pattern, which a function expression wouldn't give us. It seems like the predicate would give you what you want though.

there is an issue with only matching errors by type caused mostly by the fact that most custom errors raised by environments (like Node) do not have meaningful types - but instead patch things like a .code property

This is interesting. Does this mean that catching errors by type isn't what we need? Another option is to reconsider pattern matching, which might look like:

try {}
catch {
    error if error.code === 503: {
    }
    error if error.code === 404: {
    }
}

What do you think?

petkaantonov commented 8 years ago

The binding identifier is the e before the colon:

catch(bindingIdentifier : e => e.code == 500)
zenparsing commented 8 years ago

@petkaantonov Ah, missed the e.

petkaantonov commented 8 years ago

Although if you also want to support instanceof predicate shorthand, then that will be quite complicated to spec. In bluebird we check if a passed in function is the Error function or a subclass of it, otherwise it considers it a predicate function.

Another option is to use separate syntax for the instanceof and predicate function catch clauses.

zenparsing commented 8 years ago

@petkaantonov Do you have a feel for what the split is between testing for type versus using a predicate? Which is more common?

RangerMauve commented 8 years ago

Personally, I only ever use predicates for testing errors. It feels more that if catching-by-type becomes a language feature, people will decide to just start using typed errors, much like how people are just stuffing every new es6 feature in their code for the sake of having it.

zenparsing commented 8 years ago

@RangerMauve would you care to paste an example?

petkaantonov commented 8 years ago

I'd say predicate. For instance DOM recently deprecated making various sub errors of FileError and now you just check e.name === "FileNotReadable" etc. In node you have to use predicates for everything since core has set the convention to add properties.

benjamingr commented 8 years ago

Oh hi @RangerMauve, nice seeing you here :)

@zenparsing in my experience people use the typed version more, but that's entirely anecdotal. I don't mind doing a sweep on packages that depend on bluebird and come back with statistics but that'll have to be next weekend. Personally I find the predicate version more useful - of course optimally it'd be really cool to see them both work.

zenparsing commented 8 years ago

@benjamingr Statistics would be really great.

RangerMauve commented 8 years ago

@zenparsing Sure, for example, in my promise-based code I'd be catching HTTP errors

something().catch(function(e){
  if(e.code === 0) displayNetworkWarning();
});

I suppose adding a predicate would make the if statement less simple. Though at that point, wouldn't the only thing we're saving on be a potential re-throw at the end of the ifs?

Like

try {
something();
} catch(e) {
  if(e.code === 0) handleError0();
  else if(e.code === 400) handleError400();
  else throw e;
}

try { something(); }
catch(e) if(e.code === 0){ handleError0(); }
catch(e) if(e.code === 400){ handleError400(); }
RangerMauve commented 8 years ago

@benjamingr I like stalking ES features these days. :P

Personally, I haven't used type based clauses in catch statements because it was a pain to work with, and because most of the libraries I've used didn't bother setting up different error types (or exposing them) for errors that occur. I personally don't use typed errors in my libraries, either.

Though if there was nice syntax for working with it, I could see it becoming more useful for people.

zenparsing commented 8 years ago

Interestingly, you can use @@hasInstace to create predicates without a dedicated predicate syntax:

function _if(pred) {
    return { [Symbol.hasInstance]: pred };
}

try {
    something();
} catch (error : _if(e => e.code === 503)) {
    handle503();
}

I'm not suggesting this is a solution, just an interesting result.

benjamingr commented 8 years ago

Yeah, I noticed it too (the hook), I thought about it differently though (defining a NetworkError whose hasInstance is the predicate on the error object - more like our behavioral typing).

I'm not sure it's a good idea either - it feels elegant and backwards at the same time.

RangerMauve commented 8 years ago

I kinda like it, but in that case this functionality would have to be implemented in user-land, so it might not be used as much and people will still have to use if statements within the catch blocks rather than having syntax for adding custom checks.

Though even without the predicate stuff it'd be useful for stuff like

try {
  something();
} catch(e : HTTPError)
  // Look at the status code and react to it
} catch(e: ADifferetnError){
  // Do something else with the properties
}

Where you'd at least be able to differentiate between errors on a higher-level.

benjamingr commented 8 years ago

For instance DOM recently deprecated making various sub errors of FileError and now you just check e.name === "FileNotReadable" etc

@petkaantonov any chance to get a link to that discussion and the DOM deprecating it?