Skip to content

Conversation

@HJLebbink
Copy link
Member

refactored errors to create a higher level Error and four lower level errors

@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch from 2267cd6 to d9516d6 Compare July 17, 2025 15:24
@HJLebbink HJLebbink added the cleanup-rewrite Used in release doc generation label Jul 17, 2025
@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch from d9516d6 to bd7ed58 Compare July 17, 2025 15:31
@HJLebbink HJLebbink requested a review from Copilot July 17, 2025 15:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the error handling system to create a hierarchical error structure, replacing a single flat Error enum with a higher-level Error enum and four specialized error types: ValidationErr, S3ServerError, ReqwestError, and IoError.

  • Error hierarchy restructuring: Replaced monolithic Error enum with specialized error types for better error categorization and handling
  • Header constant consolidation: Centralized HTTP header constants in a dedicated module to improve consistency and maintainability
  • Testing improvements: Enhanced test structure with helper functions and additional test cases for edge cases like whitespace handling

Reviewed Changes

Copilot reviewed 131 out of 131 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/s3/error.rs Complete restructure of error types into hierarchical system
src/s3/header_constants.rs New module containing all HTTP header constants
src/s3/minio_error_response.rs New module for S3-specific error responses and error codes
tests/* Multiple test files refactored with helper functions and additional test cases
src/s3/utils.rs Updated utility functions to use new error types and URL encoding
Comments suppressed due to low confidence (1)

src/s3/utils.rs:27

  • This type alias is defined but never used in the diff. Consider removing it if it's not needed.
use md5::compute as md5compute;

@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch 2 times, most recently from 62764d7 to b70df3e Compare July 17, 2025 15:53
@HJLebbink HJLebbink marked this pull request as ready for review July 17, 2025 15:59
@HJLebbink HJLebbink requested review from donatello and twuebi July 17, 2025 15:59
@HJLebbink HJLebbink self-assigned this Aug 11, 2025
@donatello
Copy link
Member

This is great! please fix conflicts.

@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch from 41a16e1 to 564bc9a Compare August 11, 2025 19:29
@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch from 84a966e to 1243a19 Compare August 11, 2025 19:52
@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch 2 times, most recently from f3a50da to 6d48b00 Compare August 11, 2025 20:31
@HJLebbink HJLebbink force-pushed the refactor/cleanup-errors branch from 6d48b00 to a894782 Compare August 12, 2025 09:36
@HJLebbink
Copy link
Member Author

@donatello fixed merge issues. PTAL

@donatello donatello merged commit 5080bf9 into minio:master Aug 15, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup-rewrite Used in release doc generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants