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.

Sign up for GitHub

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

Jump to bottom

refactor(error): enhance error logging and output formatting #361

Merged
merged 4 commits into from
Nov 18, 2024
Merged

refactor(error): enhance error logging and output formatting #361

merged 4 commits into from
Nov 18, 2024

Conversation

simonsan
Copy link
Contributor

@simonsan simonsan commented Nov 18, 2024 •
edited
Loading

  • adds display_log() to RusticError for a minimal display to be used in logging
  • uses above method in error!/warn!/info! macros over the codebase
  • added snapshot test to make sure the output is as expected

@simonsan simonsan added A-errors Area: error handling needs improvement C-enhancement Category: New feature or request A-dx Area: Related to Developer Experience C-refactor Category: Refactoring of already existing code labels Nov 18, 2024
@simonsan simonsan added this to the NEXT milestone Nov 18, 2024
nardoor
nardoor previously approved these changes Nov 18, 2024
View reviewed changes
Copy link
Contributor

@nardoor nardoor left a comment

Choose a reason for hiding this comment

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

Hey,

I added a suggestion for the display_log impl.
Wether you take it or not the code lgtm!

I see value in having synthetic log for some errors.

Just a thought/question:

  • if we use display_log more than the impl Display: could we want to invert both implementations ? (display_full and impl Display { fn display(&self) { self.display_log() } }

crates/core/src/commands/check.rs Show resolved Hide resolved
crates/core/src/error.rs Outdated Show resolved Hide resolved
@simonsan
Copy link
Contributor Author

  • if we use display_log more than the impl Display: could we want to invert both implementations ? (display_full and impl Display { fn display(&self) { self.display_log() } }

Hey, no I think we use it way less. We use the errors in over 140 places, and the logs in just a few. Also we use this just now, because we log the error, e.g. in check, which will be refactored next. So I think the error Display should stay as is.

@simonsan simonsan merged commit 27bbbbd into main Nov 18, 2024
20 of 21 checks passed
@simonsan simonsan deleted the refactor/error-log-output branch November 18, 2024 18:43
@rustic-release-plz rustic-release-plz bot mentioned this pull request Nov 18, 2024
Merged
simonsan pushed a commit that referenced this pull request Nov 18, 2024
@rustic-release-plz
chore: release (#354)
7a3b68e 
## 🤖 New release
* `rustic_backend`: 0.4.2 -> 0.5.0 (⚠️ API breaking changes)
* `rustic_core`: 0.5.5 -> 0.6.0 (⚠️ API breaking changes)
* `rustic_testing`: 0.2.3 -> 0.3.0 (✓ API compatible changes)

### ⚠️ `rustic_backend` breaking changes

```
--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/enum_missing.ron

Failed in:
  enum rustic_backend::error::LocalBackendErrorKind, previously in file /tmp/.tmp0sSY8G/rustic_backend/src/error.rs:90
  enum rustic_backend::error::RestErrorKind, previously in file /tmp/.tmp0sSY8G/rustic_backend/src/error.rs:67
  enum rustic_backend::error::BackendAccessErrorKind, previously in file /tmp/.tmp0sSY8G/rustic_backend/src/error.rs:10
  enum rustic_backend::error::RcloneErrorKind, previously in file /tmp/.tmp0sSY8G/rustic_backend/src/error.rs:43

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/module_missing.ron

Failed in:
  mod rustic_backend::error, previously in file /tmp/.tmp0sSY8G/rustic_backend/src/error.rs:1
```

### ⚠️ `rustic_core` breaking changes

```
--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/inherent_method_missing.ron

Failed in:
  LocalDestination::remove_dir, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:129
  LocalDestination::remove_file, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:152
  LocalDestination::create_dir, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:171
  LocalDestination::set_times, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:189
  LocalDestination::set_user_group, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:237
  LocalDestination::set_uid_gid, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:283
  LocalDestination::set_permission, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:324
  LocalDestination::set_extended_attributes, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:385
  LocalDestination::set_length, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:466
  LocalDestination::create_special, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:521
  LocalDestination::read_at, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:599
  LocalDestination::write_at, previously in file /tmp/.tmp0sSY8G/rustic_core/src/backend/local_destination.rs:663
  RusticError::into_inner, previously in file /tmp/.tmp0sSY8G/rustic_core/src/error.rs:46
  RusticError::backend_error, previously in file /tmp/.tmp0sSY8G/rustic_core/src/error.rs:61
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `rustic_backend`
<blockquote>

##
[0.5.0](rustic_backend-v0.4.2...rustic_backend-v0.5.0)
- 2024-11-18

### Added

- *(async)* add `async_compatible` methods to identify backend
compatibility
([#355](#355))
- add 'yandex-disk' to enabled opendal services and update opendal to
0.50.2 ([#360](#360))

### Other

- *(error)* enhance error logging and output formatting
([#361](#361))
- *(backend)* simplify code in local backend
([#362](#362))
- *(backend)* migrate from `backoff` to `backon`
([#356](#356))
- *(error)* improve error messages and file handling
([#334](#334))
- *(deps)* lock file maintenance rust dependencies
([#345](#345))
- *(deps)* [**breaking**] upgrade to new conflate version
([#300](#300))
- *(errors)* [**breaking**] Improve error handling, display and clean up
codebase ([#321](#321))
</blockquote>

## `rustic_core`
<blockquote>

##
[0.6.0](rustic_core-v0.5.5...rustic_core-v0.6.0)
- 2024-11-18

### Added

- *(async)* add `async_compatible` methods to identify backend
compatibility
([#355](#355))

### Fixed

- prevent overwriting hot repository on init
([#353](#353))

### Other

- *(error)* enhance error logging and output formatting
([#361](#361))
- *(deps)* remove Derivative and replace with Default impl due to
RUSTSEC-2024-0388
([#359](#359))
- *(error)* improve error messages and file handling
([#334](#334))
- *(deps)* lock file maintenance rust dependencies
([#345](#345))
- *(deps)* remove cdc and switch to rustic_cdc
([#348](#348))
- *(deps)* [**breaking**] upgrade to new conflate version
([#300](#300))
- *(errors)* [**breaking**] Improve error handling, display and clean up
codebase ([#321](#321))
</blockquote>

## `rustic_testing`
<blockquote>

##
[0.3.0](rustic_testing-v0.2.3...rustic_testing-v0.3.0)
- 2024-11-18

### Added

- *(async)* add `async_compatible` methods to identify backend
compatibility
([#355](#355))

### Other

- *(errors)* [**breaking**] Improve error handling, display and clean up
codebase ([#321](#321))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Co-authored-by: rustic-release-plz[bot] <182542030+rustic-release-plz[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nardoor nardoor nardoor left review comments

@aawsome aawsome Awaiting requested review from aawsome

Assignees
No one assigned
Labels
A-dx Area: Related to Developer Experience A-errors Area: error handling needs improvement C-enhancement Category: New feature or request C-refactor Category: Refactoring of already existing code
Projects
None yet
Milestone
NEXT
Development

Successfully merging this pull request may close these issues.
2 participants
@simonsan @nardoor