[WIP] feat: add errors with codes and utils

[hugomrdias]

This PR adds a proposal to improve the error handling beyond using err-code, with these changes we create static errors backed by error classes and printf style formatted messages.
This approach is a simplified version of node's errors https://github.com/nodejs/node/blob/master/lib/internal/errors.js, i guess if it's good for node's code base it should also be good for us.

The benefits :

• "standard" being based on node's version we can assume it works well in a large code base like ours
• errors are static we require them instead of just adding a string as a code property, this is less error prone
• messages are more stable because they are defined with the error and error code
• messages output is more flexible and advanced, it uses node util.format so things like objects and json use util.inspect for a nicer output.
• errors are sub-classes of native primitive error type this allows for more complex error matching
• we can match using error codes e.g. err.code === ERR_IPFS_MISSING_IPLD_FORMAT.code
• errors are directly associated with error messages, this gives the dev more context about the error use case which in turn will lead to a better error selection.

Please give feedback about this PR if we all agreed with this i'll change the rest of code to this pattern. //cc @ipfs/javascript-team

[vmx]

I really like to have better errors. When we roll out proper errors, we might want to consider adding a cause field. For more information see e.g. https://www.joyent.com/node-js/production/design/errors#5-if-you-pass-a-lower-level-error-to-your-caller-consider-wrapping-it-instead (scroll up a bit, anchors don't work well with the current website design). The whole article is a good read.

You can use cause to nest errors and always have access to the original one. I think of cases where e.g. the datastore can't open a directory and you want to bubble that up but nicely wrapped in a proper IPFS error message. Being able to nest errors is also something that modern languages like Rust support.

And as I stumbled upon another great read recently, I'll also put that one out here: http://250bpm.com/blog:140

[mcollina]

Big +1 on this, if follows Node core approach.

Note that in readable-stream we had to do a custom porting of the logic to support old browsers.
See https://github.com/nodejs/readable-stream/blob/master/errors-browser.js https://github.com/nodejs/readable-stream/blob/master/errors.js.

I think it's preferrable to not use a map and just use an object.

Docs for the error codes are missing.

[vasco-santos]

I also agree with moving on with this approach for the errors.

My biggest concern is that with so many modules, I don't think that the best approach is to have errors defined in all those modules and export them so that they can be used by the others. At least, we should have the more generic errors in their own module.

[alanshaw]

I like this. We do need to deal with the case where we get an error from a dependency and want to "wrap it" to preserve the original (as @vmx noted). This proposal doesn't allow us to do that right now but err-code does.

@vmx - my concern with adding a cause property is that it gets hidden in browser console output:

Chrome (cannot see cause in throw or log):

Firefox (cannot see cause in throw or log):

Node.js (it's ok, but we don't see the stack from wrapper...not entirely sure that matters though!):

@hugomrdias as a suggesstion, explain-error actually preserves the error and might give you the basis for a proposal to allow us to support this...

Another idea, if @vmx, @vasco-santos and @jacobheun are on board with this we should pull the error creation code out into a module so that IPLD and libp2p can also use it.

[vasco-santos]

Another idea, if @vmx, @vasco-santos and @jacobheun are on board with this we should pull the error creation code out into a module so that IPLD and libp2p can also use it.

I am totally on this

[vmx]

Another idea, if @vmx, @vasco-santos and @jacobheun are on board with this we should pull the error creation code out into a module so that IPLD and libp2p can also use it.

That would be cool

[vmx]

@vmx - my concern with adding a cause property is that it gets hidden in browser console output:

I'm more concerned about having the information be stored somewhere. When you use a debugger, you can still access it. If we can make it even show up in the Browser console that would be even better.

[vmx]

Another things I came across while working on some code. Let's assume you delete several items by CID, but that's not always successful, so you might get an error message. You would want to know on which CID it failed.

You can add this information to the error message text, but you won't have programmatic access to it.

So what about changing the fprint like formatting strings (where arguments are an array) to more Template Literal-like ones (where arguments are named).

Example:

// Instead of
new ERR_IPFS_MISSING_IPLD_FORMAT(codec)
// You would do 
new ERR_IPFS_MISSING_IPLD_FORMAT({ codec: codec })
// Or if you're fancy 
new ERR_IPFS_MISSING_IPLD_FORMAT({ codec })

The format string would be:

// Instead of
'Missing IPLD format %s'
// You'd be using
`Missing IPLD format ${args.codec}`

The advantage would be that the error object itself could have a field called e.g. args, context or values where you have access to the properties you passed in. This way you would have programmatic access to the custom things that caused the error.

[AuHau]

Is there a plan to continue with this? I think that there is a big improvement space for error handling in js packages and this would be a great step forward.

I would also suggest having a one master table where all error codes would be collected from all js packages. Maybe together which packages can throw it? (but that could be hard to maintain, yet it could be automatized)

[alanshaw]

@hugomrdias do you plan to move forwards with this or shall we close?

[hugomrdias]

i plan to include this in this quarter work

[jacobheun]

Closing as stale