Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: error objects and handling #203

Closed
wants to merge 1 commit into from
Closed

Conversation

AuHau
Copy link
Member

@AuHau AuHau commented Aug 7, 2019

This refactors style of error objects that are thrown and that follows style proposed in ipfs/js-ipfs#1746

I would say big plus of this PR is that it moves all errors to ./errors and hence create overviews of errors that are thrown from ipfs-repo.

@jacobheun I am not so sure about this not introducing breaking change any more. The thrown errors indeed contain code property as mentioned earlier, but the problem is how to assert the code's value. Earlier you could do with some errors:

const errors = require('ipfs-repo').errors

if(err.code === errors.ERR_REPO_ALREADY_OPEN)
...

now you have to do:

const errors = require('ipfs-repo').errors

if(err.code === errors.ERR_REPO_ALREADY_OPEN.code)
...

Which packages uses js-ipfs-repo? I assume js-ipfs? I will create PR to reflects this change there, but should I look somewhere else?

License: MIT
Signed-off-by: Adam Uhlir <[email protected]>
@AuHau AuHau requested a review from jacobheun August 7, 2019 09:55
@@ -1,5 +1,7 @@
'use strict'

// TODO: Should be refactored when https://github.com/ipfs/js-ipfs/pull/1746/ is adopted.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PR there are some utils to ease the creation of the Error's class, but since I believe it will be moved out to some general package to be used across js land, I did not copy&pasted it directly here and just created the Error's classes manually. Once it will land this can be adopted here.

@jacobheun
Copy link
Contributor

@hugomrdias can you weigh in here since you're looking at changes in js-ipfs errors. The error exports are going to be a breaking change. If we're breaking the api, I'd like to avoid doing so more than once with these updates.

@AuHau
Copy link
Member Author

AuHau commented Aug 12, 2019 via email

@jacobheun
Copy link
Contributor

Closing this until error code handling is determined in js-ipfs ipfs/js-ipfs#1746

@jacobheun jacobheun closed this Jun 18, 2020
@achingbrain achingbrain deleted the refactor/error_objects branch September 13, 2021 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants