search

ghcjs / ghcjs-base

base library for GHCJS for JavaScript interaction and marshalling, used by higher level libraries like JSC
MIT License
45 stars 67 forks source link

Use informative exceptions instead of Maybe for fromJSRef #24

Open mgsloan opened 9 years ago

mgsloan commented 9 years ago

Currently, the type of improved-base fromJSRef is fromJSRef :: FromJSRef a => JSRef -> Maybe a. Having a Maybe type for marshaling failures isn't very informative, and in practice, I find that I always want the Nothing case to be a failure. By using the Monad instance for Maybe, we lose info about why and where marshaling failed. Each time I use fromJSRef, I need to write my own error message saying that overall marshaling failed.

How can we resolve this? My proposal is to instead throw an exception. Here's why:

1) It's more efficient than plumbing Maybe everywhere. 2) fromJSRef already lives in IO. 3) I almost always want marshaling failure to be an exception. 4) In the cases where I want to do something upon marshaling exception, I can catch the exception.

I think the following is a reasonable type for exceptions during marshaling:

newtype FromJSRefException a = FromJSRefException a
  deriving (Typeable, Show)

instance (Show a, Typeable a) => Exception (FromJSRefException a)

Ideally, there would be a convention of always throwing exceptions wrapped in this, to provide a uniform way of catching marshaling issues. It may also be worth considering passing through a path like [(TypeRep, String)], so that it's clear where in the structure the error is occurring. It might be good to make this optional, though, as there could be quite a bit of overhead.

mgsloan commented 9 years ago

I discussed this some with Luite at ICFP, and it sounds like the most efficient way to get info about where the marshalling error occurred is actually to wrap things in a catch at all the spots we want to extend the error with more info.

mgsloan commented 8 years ago

One complication here is that it would be consistent for the pure version to also throw marshaling exceptions. However, they may not get forced during marshaling, and so wouldn't get the path information indicating the path to the issue.

Maybe this is a good reason to explicitly build the path rather than doing it with exception catching? I don't like that it adds complexity to the API, but I don't see a better way. Except perhaps introducing MarshalT, heh