-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the one multihash error hiding I found, I think this looks good.
Ideally we'd have the error codes in a single place to avoid typos. I know there were discussions in #1406 about centralizing those in their own repo. Perhaps we could add them to a single file in here for the interim?
This is a great step in the right direction for better error handling! 🎉
src/core/components/object.js
Outdated
} catch (err) { | ||
return callback(err) | ||
return setImmediate(() => callback(errCode(err, 'ERR_INVALID_CID'))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If normalizeMultihash
throws the error users are going to get confused since this will always say the cid is the problem. We should check if it's an unsupported hash error first.
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
License: MIT Signed-off-by: Alan Shaw <[email protected]>
774a5c3
to
ec24044
Compare
@jacobheun can I get a 👍 now? (will also wait for a jenkins 💚 too) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending CI, 👍 looks good
@diasdavid wanted to double check this was ok by you? |
@alanshaw what's the pros/cons of err-code that you see? |
Pros:
Note that when I mention "user" above I'm referring to developers using js-ipfs. Cons:
Additionally, I'm aware of the following difficulties in introducing this:
|
This PR adds a variety of fixes and tests for invalid inputs passed to the IPFS node that would crash the daemon.
The module
err-code
is used to add code's to error messages that the client can rely on and we can use to assert on in our tests.refs #1406
also sent as part of this work libp2p/js-libp2p#232
related to https://github.com/ipfs/interface-ipfs-core/issues/300