fpco / inline-c

284 stars 51 forks source link

Preserve exception #130

Closed roberth closed 2 years ago

roberth commented 2 years ago

Currently, exceptions are caught by the Cpp.Exceptions module, where the original exception is thrown away. While this can be circumvented at a small scale by catching certain exceptions in C++ code, this is not feasible at a large scale, as it goes against what exceptions are intended for: make error handling implicit and centralize it around a few high-level call sites.

This PR fixes this problem by using std::exception_ptr to represent C++ exceptions as first-class Haskell values.

As a result, the caller can write their own exception handling function, to extract valuable information from specific exceptions. For example:

renderException :: SomeException -> IO Text
renderException e | Just (C.CppStdException ex _msg _ty) <- fromException e = renderStdException ex
-- ...

renderStdException :: C.CppExceptionPtr -> IO Text
renderStdException e =
  [C.throwBlock| char * {
    std::string r;
    std::exception_ptr *e = $fptr-ptr:(std::exception_ptr *e);
    try {
      std::rethrow_exception(*e);
    } catch (const somelib::Error &e) {
      std::stringstream s;
      // obtain detailed exception info
      somelib::showErrorInfo(s, e.info());
      r = s.str();
    } catch (const std::exception &e) {
      r = e.what();
    }
    return strdup(r.c_str());
  }|] >>= unsafePackMallocCString <&> decodeUtf8With lenientDecode

Review and implementation notes

The second commit renames a file, making the combined diff unhelpful. The first commit contains all the actual changes.

I've decided to move the code from Exceptions to Exception so that

I've decided to keep the string representations of the message and exception type, in order to avoid calling C++ code in the Show instance. This can be a little inefficient, but much better than storing these in [Char] like we did.

I've decided to make the exception module(s) depend on C++11. I believe requiring a decade old standard should not pose a problem. The alternative is to revive the old code as an independent module, but until we know that such a maintenance overhead is useful to anyone, this would be a wasted effort and a liability.

bitonic commented 2 years ago

Thanks!

roberth commented 2 years ago

Could you make a release? This one can be breaking, so inline-c-cpp needs a bump 0.4.x.y -> 0.5.0.0. inline-c is still equal to its released version, so it's only about inline-c-cpp.

bitonic commented 2 years ago

Embarassingly, my Haskell setup is currently broken somehow (looks like this https://discourse.nixos.org/t/stack-build-fails-with-startprocess-invalid-argument-bad-file-descriptor/1668 ). I'll do a release as soon as I have a second to fix it.

roberth commented 2 years ago

This seems to work (if you excuse my unpinned <nixpkgs>; it's the latest nixos-21.11)

{ pkgs ? import <nixpkgs> {} }:

pkgs.mkShell {
  nativeBuildInputs = [
    pkgs.stack
    pkgs.ghc
    pkgs.cabal-install
    pkgs.haskellPackages.haskell-language-server
    pkgs.haskellPackages.hie-bios
  ];
}
nix-shell -I nixpkgs=channel:nixos-21.11 shell.nix

If that doesn't solve it, I've had some success clearing the ~/.stack and $(find -name .stack-work) directories by hand. Possibly because stack produces store references but no gc roots. Not sure if it solves your error, but errno can be deceptive.

bitonic commented 2 years ago

Done, removing ~/.stack did it