jabber-tools / cognitive-services-speech-sdk-rs

Apache License 2.0
24 stars 15 forks source link

Implement std::error::Error trait for error::Error struct. #16

Closed wangfu91 closed 5 months ago

wangfu91 commented 5 months ago

This PR made one small change: Impl stdError trait for the error::Error struct, so that it can work well with the anyhow::Result<T> return type. e.g.

pub async fn transcribe_speaker_output(&self) -> anyhow::Result<()> {
   let wave_format = AudioStreamFormat::get_wave_format_pcm(16000, Some(16), Some(1))?;
   let mut pull_input_stream = PullAudioInputStream::from_format(&wave_format)?;
   let audio_config = AudioConfig::from_stream_input(&pull_input_stream)?;
   ...
}
adambezecny commented 5 months ago

hi again:)

should be fine but give me some time pls. I want to prioritise some other changes before this and do not want to publish new version with such a small change. Thanks for your patience!

regards,

Adam

adambezecny commented 5 months ago

hi,

https://github.com/jabber-tools/cognitive-services-speech-sdk-rs/pull/15 will be ready for merge soon. I would like to ideally merge your PR together with it, so that we don't create two versions. But there is still one pending item (see above impl std::error::Error for Error {}). Can you provide the answer please?

regards,

Adam

wangfu91 commented 5 months ago

But there is still one pending item (see above impl std::error::Error for Error {}). Can you provide the answer please?

Sorry, could you be more specific? I think I already addressed the pending issue.

adambezecny commented 5 months ago

i was asking this: question about impl std::error::Error for Error {}: defining trait without actual method implementation is good for what? Or in other words: how it is intended to work exactly?

see above. Just want to understand how is this empty trait implementation meant and what is it supposed to do actually.

wangfu91 commented 5 months ago

Just want to understand how is this empty trait implementation meant and what is it supposed to do actually.

Based on what I read here: Custom errors and implementing the Error trait, the methods in std::error::Error trait already have default implementations, and the std::fmt::Display trait is already implemented and will print the user-friendly error message.

However, I'm now not sure whether we should implement the source method of the Error trait ourselves. Given that we have a caused_by field which seems to signify the underlying cause of the error, it might be beneficial to return this low-level error from the source method. I'm open to further discussion or guidance on this matter.

adambezecny commented 5 months ago

I think source + caused_by is ok. merging. this one will be published together with PR17. thanks!

adambezecny commented 5 months ago

hi,

PR17 is taking longer than expected to finish so I published version 1.0.1 (with PR16) in the meantime. Enjoy! :-)