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): eliminate usage of DisplayErrorContext #15225

Merged
merged 2 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ disallowed-types = [
{ path = "num_traits::FromPrimitive", reason = "Please use `From` or `TryFrom` with `OrderedFloat` instead." },
{ path = "num_traits::ToPrimitive", reason = "Please use `From` or `TryFrom` with `OrderedFloat` instead." },
{ path = "num_traits::NumCast", reason = "Please use `From` or `TryFrom` with `OrderedFloat` instead." },
{ path = "aws_smithy_types::error::display::DisplayErrorContext", reason = "Please use `thiserror_ext::AsReport` instead." },
]
disallowed-macros = [
{ path = "lazy_static::lazy_static", reason = "Please use `std::sync::LazyLock` instead." },
Expand Down
23 changes: 5 additions & 18 deletions src/connector/src/sink/kinesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

use std::collections::HashMap;

use anyhow::anyhow;
use aws_sdk_kinesis::error::DisplayErrorContext;
use anyhow::{anyhow, Context};
use aws_sdk_kinesis::operation::put_record::PutRecordOutput;
use aws_sdk_kinesis::primitives::Blob;
use aws_sdk_kinesis::Client as KinesisClient;
Expand Down Expand Up @@ -106,10 +105,8 @@ impl Sink for KinesisSink {
.stream_name(&self.config.common.stream_name)
.send()
.await
.map_err(|e| {
tracing::warn!("failed to list shards: {}", DisplayErrorContext(&e));
Copy link
Member Author

Choose a reason for hiding this comment

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

There should be a principle that "whoever handles the error is responsible for logging the error". The error here is thrown and should be handled (or logged) by the caller.

SinkError::Kinesis(anyhow!("failed to list shards: {}", DisplayErrorContext(e)))
})?;
.context("failed to list shards")
.map_err(SinkError::Kinesis)?;
Ok(())
}

Expand Down Expand Up @@ -201,18 +198,8 @@ impl KinesisSinkPayloadWriter {
},
)
.await
.map_err(|e| {
tracing::warn!(
"failed to put record: {} to {}",
DisplayErrorContext(&e),
self.config.common.stream_name
);
SinkError::Kinesis(anyhow!(
"failed to put record: {} to {}",
DisplayErrorContext(e),
self.config.common.stream_name
))
})
.with_context(|| format!("failed to put record to {}", self.config.common.stream_name))
.map_err(SinkError::Kinesis)
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/connector/src/source/filesystem/s3_v2/lister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use anyhow::anyhow;
use anyhow::Context;
use async_trait::async_trait;
use aws_sdk_s3::error::DisplayErrorContext;
use aws_sdk_s3::types::Object;
use itertools::Itertools;

Expand All @@ -39,7 +38,7 @@ impl FsListInner for S3SplitEnumerator {
let mut res = req
.send()
.await
.map_err(|e| anyhow!(DisplayErrorContext(e)))?;
.with_context(|| format!("failed to list objects in bucket `{}`", self.bucket_name))?;
if res.is_truncated().unwrap_or_default() {
self.next_continuation_token = res.next_continuation_token.clone();
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/connector/src/source/kinesis/source/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

use std::time::Duration;

use anyhow::anyhow;
use anyhow::{anyhow, Context};
use async_trait::async_trait;
use aws_sdk_kinesis::error::{DisplayErrorContext, SdkError};
use aws_sdk_kinesis::error::SdkError;
use aws_sdk_kinesis::operation::get_records::{GetRecordsError, GetRecordsOutput};
use aws_sdk_kinesis::primitives::DateTime;
use aws_sdk_kinesis::types::ShardIteratorType;
Expand Down Expand Up @@ -243,7 +243,7 @@ impl KinesisSplitReader {
.set_timestamp(starting_timestamp)
.send()
.await
.map_err(|e| anyhow!(DisplayErrorContext(e)))?;
.context("failed to get kinesis shard iterator")?;

if let Some(iter) = resp.shard_iterator() {
Ok(iter.to_owned())
Expand Down
3 changes: 1 addition & 2 deletions src/meta/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use aws_sdk_ec2::error::DisplayErrorContext;
use risingwave_common::error::BoxedError;
use risingwave_connector::error::ConnectorError;
use risingwave_connector::sink::SinkError;
Expand Down Expand Up @@ -103,7 +102,7 @@ pub enum MetaErrorInner {
SinkError,
),

#[error("AWS SDK error: {}", DisplayErrorContext(& * *.0))]
#[error("AWS SDK error: {0}")]
Aws(#[source] BoxedError),

#[error(transparent)]
Expand Down
3 changes: 1 addition & 2 deletions src/object_store/src/object/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use std::io;
use std::marker::{Send, Sync};

use aws_sdk_s3::error::DisplayErrorContext;
use aws_sdk_s3::operation::get_object::GetObjectError;
use aws_sdk_s3::operation::head_object::HeadObjectError;
use aws_sdk_s3::primitives::ByteStreamError;
Expand All @@ -28,7 +27,7 @@ use tokio::sync::oneshot::error::RecvError;
#[derive(Error, Debug, thiserror_ext::Box, thiserror_ext::Construct)]
#[thiserror_ext(newtype(name = ObjectError, backtrace, report_debug))]
pub enum ObjectErrorInner {
#[error("s3 error: {}", DisplayErrorContext(&**.0))]
#[error("s3 error: {0}")]
S3(#[source] BoxedError),
#[error("disk error: {msg}")]
Disk {
Expand Down
Loading