Skip to content

Instantly share code, notes, and snippets.

@marianoguerra
Forked from anonymous/error.erl
Last active October 9, 2016 19:23
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save marianoguerra/dc2e0aa73964f0e501f4b4433368ee00 to your computer and use it in GitHub Desktop.
Save marianoguerra/dc2e0aa73964f0e501f4b4433368ee00 to your computer and use it in GitHub Desktop.
Standard Error Format for Erlang
-record(error, {type, ctx}).
% TODO: macros to create error adding module, line and function to context
% TODO: line(#error{}), module(#error{}), function(#error{}) to return those fields if defined or undefined if not
% TODO: pretty printer
new(Type) -> new(Type, #{}).
new(Type, Ctx) -> #error{type=Type, ctx=Ctx}.
wrap(Type, Cause=#error{}) -> wrap(Type, Cause, Ctx).
wrap(Type, Cause=#error{}, Ctx) ->
#error{type=Type, ctx=maps:merge(Ctx, #{cause => Cause})}.
cause(#error{ctx=#{cause := Cause}}) -> {ok, Cause};
cause(#error{}) -> notfound.
get(Error=#error{}, Key) -> get(Error, Key, undefined).
get(Error=#error{ctx=Ctx}, Key, Default) -> maps:get(Key, Ctx, Default).
@marianoguerra
Copy link
Author

cause should return Cause or undefined like other accessors.

@marianoguerra
Copy link
Author

?ERROR_HERE macro to add location info
optional error:here/1 and error:here/2 parse transforms

@marianoguerra
Copy link
Author

also add dialyzer types and annotations.

@andytill
Copy link

andytill commented Oct 4, 2016

Requiring proplists is probably an unreasonable request because it means that pattern matching is never going to be safe. Also a 3-element tuple is unfamiliar and will break traditional {error,_} matches.

I'm going to try and work this back from use cases in my code and see how this looks.

@jadeallenx
Copy link

Agree that {error, SomeTerm} would be a better return value. I would suggest that SomeTerm ought to be a record. They have sweet syntactic sugar and work on Erlangs before 17.

@andytill
Copy link

andytill commented Oct 9, 2016

Just for anyone new that is interested, the reason/history for this...

Using maps allows loose "types" of errors, so all errors share some keys but specialisations can have keys for their own keys for context. I am not a type theorist so excuse my terminology. Idea started in this thread.

https://twitter.com/andy_till/status/781766868189065217

In examples, we have error tuples like this.

{error,{cannot_downgrade,_}} = {error, {cannot_downgrade, Version}}.

This breaks down when we need to add more context.

{error,{cannot_downgrade,_}} = {error, {cannot_downgrade, Version, Table}}. %% <== kaboom

Onto what I was envisioning...

Using maps for error context, matching could look like this.

{error, #{ type => cannot_downgrade }}

If we had a standard set of keys for basic errors and extra ones for specific errors then apps could interact with each others error types. Within an app, it would not be necessary to create new types most of the time, especially if the error message was included in the map. This is a good thing because erlang does not handle types very well.

I originally thought that it would be nice to use maps or proplists, I am not so sure now because having either would mean that matching is more complicated forever by having to handle both.

I'm also against using a record because it requires a header file. If apps are to interact with each other's error types then they would all need to include the einfo dep and use the same version. This causes build issues in the same way that edown or meck does, those apps find their way into everything and cause version conflicts.

Sharing a record may also cause issues in the future when you want to change the record by adding fields and it can no longer be understood by other parts of the running code. For example in one VM it is probably ok but once it is passed around the cluster it will cause bad record errors.

My use case for these errors is to handle "expected errors", for example someone not being able to find a table because the user spelt it wrong. You could still think of this as expected behaviour, even as the success path. For this reason, I do not need to know the module, function or line number. If that is needed an exception should be thrown, e.g. error/1. That might simplify things a little.

One thing to think about when writing a parse transform is that the community is very unlikely to accept it.

Currently I'm thinking of writing some dialyzer specs as the only code I would ship for standard errors, these could be included as a dep or copied into the app because deps are hard.

@marianoguerra
Copy link
Author

for people joining the conversation my latest code is here: https://github.com/marianoguerra/einfo

@andytill I made it a record because you mentioned R16 compat and because of @mrallen1's comment, I can change it to generate a map in a couple of minutes.

Regarding the fields, the ones in the record are quite standard and not supposed to change much, any other field could go in extra, we can brainstorm for some extra fields to add on #einfo{} and then seal it forever if we don't move to a map.

I would prefer it to be a map with standardized fields and use the parse transform for convenience, if people don't like the parse transform they can still use the macros and get the same result, or use functions and loose the location info.

Regarding the location fields, it's runtime free (except space), it's nice extra context to go look at and can be useful in the case where the error comes from a case/function clause not matching this error somewhere upstream but not your code and you get just "exception error: no case clause matching {error,badarg}", this would tell you where the error actually was generated and gives you an idea of what may have caused it.

just some ideas, I'm open to any kind of change as long as we get something usable that helps troubleshoot weird errors :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment