-
Notifications
You must be signed in to change notification settings - Fork 7
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
Streaming API #46
Streaming API #46
Conversation
agent/src/errors.rs
Outdated
impl convert::From<Error> for io::Error { | ||
fn from(e: Error) -> io::Error { | ||
// @todo Return whole error chain | ||
io::Error::new(io::ErrorKind::Other, e.description()) |
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.
Is it necessary to only keep the description here and throw away the error considering that the second argument to io::Error::new
is generic? Won't we loose some information that might be useful for debugging?
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.
Funny, I'm just working on these errors. This was part laziness, knowing that I was coming back to it once it was working, and partly an issue with error-chain
. Basically error-chain::Error
doesn't derive Sync
, which is required by io::Error
, so we can't chain the whole error. See this outstanding PR: rust-lang-deprecated/error-chain#163
I just became aware of error_chain::ChainedError
, which will give us the whole chain as a formatted string. Not as good as chaining the actual Error
, but at least we don't lose the error chain. I'll tack this onto the current PR. Should take me an hour or so to finish up.
Awesome! Congratulations on the rewrite. |
Thanks mate :) A long way from finished, but hopefully it'll be a lot more flexible (and easier to setup!!) for people than the previous version. |
Look at core/examples/basic.rs for streaming usage.
cc @rushmorem