Taking control of Error Handling, reducing js-ipfs Daemon crashes

[fsdiogo]

Hey everyone, this issue has the purpose of gathering everything related to the js-ipfs daemon crashes and error handling.

Right now the daemon is unstable, there are a lot of uncaught exceptions and they crash the process when an error occurs down the stack.

There is a lot of work to be done around the robustness of the daemon and gateway, so this will serve as a roadmap to the state of each problem.

Roadmap

The related issues will be tracked using the table below.

Each one of them will have a priority, the same as our labels, from P0 - Critical to P4 - Very Low.

If possible, instructions of how to manually reproduce the error/crash and then tests to cause those errors, so we can track down the problem and start working on it.

When that problem is solved we'll link the solution or PR here, and mark the Status column as solved.

Issue	Description	Priority	Manual Reproduction	Test Reproduction	Fix	Status
#1326	WebSocket connection error	P1				❌
#1325 #1156	Already piped	P1			#1458	✅
#1331	Invalid block	P2			#1378	✅

Improving debuggability

We should strive to make debugging as smooth as possible.

Using debug to log meaningful events, namespacing the module you are on, is super helpful. It's easier to track logs and check the specific module that is writing them.

Usage is pretty straightforward too:

const debug = require('debug')
const log = debug('libp2p:switch')

log('adding WebsocketStar')

For more info, check the usage section.

Improving error handling

We should use error codes like Node does.

Check this article for reference.

❌ This is bad:

const err = new Error('some error message')

// ... 

if (err.message.match(/some error message/)) {
  // do something
}

✅ This is good:

const ERRORS = require('./errors')

const err = Object.assign(new Error('some error message'), {
  code: ERRORS.ERR_SOME_ERROR_CODE,
  path: node._repo.path
})

// ...

if (err.code === 'ERR_SOME_ERROR_CODE') {
  // do something
}

Contribute

You can help move things forward by reporting issues and being diligent on how to reproduce the errors. After opening an issue, if you think it's fit for being here, comment providing the link and I'll add it to the table.

Also, feel free to dig in and try to tackle some of them, help is appreciated 🙂

Reference

• Deploying js-ipfs to the Galaxy #804
• Better error handling in daemon + gateway #1325

[fsdiogo]

Would it be worth it to have a repo that listed all the error codes in the js-ipfs scope?

It would ease the process of finding the correct codes when we need to use a code that is in a different repo.

The codes would then be imported from that repo.

[vasco-santos]

I was thinking the same while doing the ipns working locally feature and more precisely here.

I added several errors that I believe that could be in a common repo and could be reused across multiple other modules.

[fsdiogo]

Yeah, I think so too. I can create that repo and start to extract some of the errors in the other repos.

What do you guys think of this?

@diasdavid @alanshaw @jacobheun @dryajov @victorbjelkholm @hugomrdias

[jacobheun]

I would love to see externalized, extendable errors across ipfs/libp2p/ipld.

I also think the errors could be a great place to use classes. Extending errors and being able to use instanceof would be extremely helpful in handling errors properly and generically (when appropriate). Making these classes and having the codes implemented in the classes themselves could also help reduce the possibility for someone to accidentally use the wrong error code.

This is something I'd really love to get integrated for libp2p and libp2p-switch in Q3, since those are two areas where errors are particularly difficult to debug.

[daviddias]

No more "already piped" with 0.31 - #1458

[alanshaw]

#1506 addresses a bunch of issues where we're not properly handling invalid user input that would cause a daemon to crash.

In that PR I added the err-code module to allow us to add string error codes attached to a code property on error objects. This is also being used in the IPNS PR (#1400) that will land soon.

Using an error code instead of error classes allows us to easily serialize and deserialize error objects without losing any information.

I'm interested to hear opinions on this - can we make it work? IMHO it's a positive step forwards and 100% better than what we have atm (nothing).

[vasco-santos]

@alanshaw I totally agree that we should go on this way as a rule for handling errors.

[alanshaw]

please read this for more context/reasoning: #1506 (comment)

[achingbrain]

Closing as this issue is very stale.