refactor(connector): migrate anyhow::Error to ConnectorError newtype

[BugenZhao]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Follow-up of #15086.

• Migrate all functions returning anyhow::Result to ConnectorResult.
  

  risingwave/src/connector/src/error.rs

  Lines 24 to 61 in de0233c

  	def_anyhow_newtype! {
  	pub ConnectorError,
  	// Common errors
  	std::io::Error => transparent,
  	// Fine-grained connector errors
  	AccessError => transparent,
  	WireFormatError => transparent,
  	ConcurrentRequestError => transparent,
  	InvalidOptionError => transparent,
  	SinkError => transparent,
  	PbFieldNotFound => transparent,
  	// TODO(error-handling): Remove implicit contexts below and specify ad-hoc context for each conversion.
  	// Parsing errors
  	url::ParseError => "failed to parse url",
  	serde_json::Error => "failed to parse json",
  	csv::Error => "failed to parse csv",
  	// Connector errors
  	opendal::Error => transparent, // believed to be self-explanatory
  	mysql_async::Error => "MySQL error",
  	tokio_postgres::Error => "Postgres error",
  	apache_avro::Error => "Avro error",
  	rdkafka::error::KafkaError => "Kafka error",
  	pulsar::Error => "Pulsar error",
  	async_nats::jetstream::consumer::StreamError => "Nats error",
  	async_nats::jetstream::consumer::pull::MessagesError => "Nats error",
  	async_nats::jetstream::context::CreateStreamError => "Nats error",
  	async_nats::jetstream::stream::ConsumerError => "Nats error",
  	icelake::Error => "Iceberg error",
  	redis::RedisError => "Redis error",
  	arrow_schema::ArrowError => "Arrow error",
  	google_cloud_pubsub::client::google_cloud_auth::error::Error => "Google Cloud error",
  	}

  • Provide default contexts for errors from external connector crates.
  • Provide ad-hoc contexts at different call sites for general error types like ParseIntError.
• Style improvements for using context and bail to wrap and construct errors.

Checklist

• I have written necessary rustdoc comments
• I have added necessary unit tests and integration tests
• I have added test labels as necessary. See details.
• I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
• My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
• All checks passed in ./risedev check (or alias, ./risedev c)
• My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

• My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

• My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

[BugenZhao]

Current dependencies on/for this PR:

• refactor(error): eliminate usage of DisplayErrorContext #15225
• feat(lints): lint format_error on anyhow::Error #15158
• refactor(connector): migrate anyhow::Error to ConnectorError newtype #15042 👈
• refactor(connector): make ConnectorError a wrapper of anyhow::Error #15086
• refactor(connector): simplify and clean-up unused variants of ConnectorError #15031
• refactor(error): clean-up direct error formatting (part 5, done) #14956
• refactor(error): move RwError from common to frontend #14953 : 1 other dependent PR (#14954 )
• refactor(connector): replace all RwError usage with anyhow temporarily #14950
• main

This stack of pull requests is managed by Graphite.

[xxchan]

LGTM!
At first I thought maybe it's not necessary to replace all anyhow::Error to the newtype (although this is easiest from the refactoring's point of view), but only need to change public APIs. But I quite like the added .context()s. This feels like a dynamic version of thiserror and thus I think we can use it more widely. 😄

[xxchan]

😄

[xxchan]

What about printing the payload here in the ctx? So that we can e.g., make the error msg in #13937 nicer

[xxchan]

Oh, it seems we can't print it without clone

[chenzl25]

LGTM!

[BugenZhao]

Merge activity

• Feb 23, 4:24 AM EST: Graphite failed to merge this pull request due to This repository has GitHub's merge queue enabled, which is currently incompatible with Graphite. Try your merge again, or report a bug if you see this consistently.