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

feat: make error clonable #554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

nathanosdev
Copy link
Member

@nathanosdev nathanosdev commented May 31, 2024

Overview
This makes candid::Error clonable, which makes it easier for downstream clients to also make their Error types cloneable.

I've made a related PR for agent-rs.

Considered Solutions
I can also work around by using Arc<T> in downstream clients.

Considerations
This shouldn't have any impact on security or performance. Anyone that was interacting directly with the Custom variant may have a breaking change since it is wrapped in an Arc<T> now.

Copy link
Contributor

@chenyan-dfinity chenyan-dfinity left a comment

Choose a reason for hiding this comment

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

This is a breaking change. It's quite labor intensive to bump the candid major version in the mono repo. So we try to avoid breaking changes as much as possible.

I can see two options: 1) we can let this PR to sleep for a while, until we make a major release (which can take quite a long time); 2) Candid error is not really structured. For the downstream users, it's enough to convert to err.to_string(), and clone as needed.

@nathanosdev
Copy link
Member Author

I agree that making a major bump just for this feature is not worth the effort, so I'm fine with leaving this to sleep and in the meantime I will just to_string() downstream.

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