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: refactor error to be more clear and maintainable #360

Merged
merged 14 commits into from
Feb 19, 2024

Conversation

PureWhiteWu
Copy link
Member

@PureWhiteWu PureWhiteWu commented Feb 18, 2024

Motivation

目前的实现具有以下问题:

  1. 错误无法类型化,失去了 Rust 类型系统的优势:所有的错误类型都转为了 anyhow::Error,虽然可以 downcast,但是无法在编译期保证类型安全
  2. 无法表达所有的语义:比如无法通过目前的类型返回 Thrift 标准定义的 ApplicationError
  3. 可扩展性、可维护性差:如新增的 BizError 只能让用户通过 anyhow::Error 返回,再在中间件中 downcast 判断,如果后续还需要新增错误类型,会极大影响可维护性
  4. 用户接口不清晰:如需要用户返回 BizError,但用户需要转换成 anyhow::Error 返回
  5. 错误语义不清晰:UserError 在 Rust 中被认为是 Result 的 Err,但是在 Thrift RPC 框架来看,被认为是成功的,也即 Result 的 Ok
  6. 不是所有错误都同时适用于 Server/Client:比如 Server 侧不应当出现 Transport 和 Protocol 错误
  7. 不明确的语义:比如 AnyhowError,并没有明确的语义,在传输过程中会被转换为 ApplicationError,但是在 Service 处理过程中无法提供有价值的信息

Solution

这个 PR 完成了以下更改:

  1. 将所有错误强类型化,通过 Rust 类型系统保证正确性、可扩展性、可维护性,防止误用,去除 anyhow::Error
  2. 明确所有错误语义,框架返回的所有错误都是应当可枚举可解释的
  3. 消除不必要存在的 variant,以最小所需集分别定义不同阶段的错误
  4. 修正用户自定义 Exception 语义为 Ok/Success,不再在 Error 中体现

具体来看,在 Server 端,无论是 Handler 还是 Service 中,错误类型为:

pub enum ServerError {
    Application(ApplicationException),
    Biz(BizError),
}

在 Client 端,错误类型为:

pub enum ClientError {
    Application(ApplicationException),
    Transport(TransportException),
    Protocol(ProtocolException),
    Biz(BizError),
}

@PureWhiteWu PureWhiteWu added A-volo-build This issue concerns the `volo-build` crate. A-volo-thrift The issue concerns the `volo-thrift` crate. labels Feb 18, 2024
@PureWhiteWu PureWhiteWu self-assigned this Feb 18, 2024
@PureWhiteWu PureWhiteWu requested review from a team as code owners February 18, 2024 07:25
@GuangmingLuo

This comment was marked as resolved.

Millione
Millione previously approved these changes Feb 19, 2024
@PureWhiteWu PureWhiteWu merged commit b66c5b5 into cloudwego:main Feb 19, 2024
13 checks passed
@PureWhiteWu PureWhiteWu deleted the refactor/error branch February 19, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-volo-build This issue concerns the `volo-build` crate. A-volo-thrift The issue concerns the `volo-thrift` crate.
Development

Successfully merging this pull request may close these issues.

3 participants