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

core: give SetGlobalDefaultError a useful Debug impl #2250

Merged
merged 2 commits into from Jul 27, 2022

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jul 27, 2022

Motivation

When a global default dispatcher has already been set, the
dispatch::set_global_default function fails with a
SetGlobalDefaultError. The fmt::Display impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the fmt::Debug impl is derived, and just looks
like this:

SetGlobalDefaultError { _no_construct: () }

which isn't particularly helpful.

Unfortunately, when unwrap()ping or expect()ing a Result, the
fmt::Debug implementation for the error type is used, rather than
fmt::Display. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.

Solution

This branch replaces the derived Debug impl with a manually
implemented one that prints the same message as the Display impl, but
formatted like a tuple struct with a single string field. This avoids
emitting Debug output that's just a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like

tracing::dispatch::set_global_default(foo).unwrap();

The mesasge now looks like:

SetGlobalDefaultError("a global default trace dispatcher has already been set")

which should be a bit easier to debug.

## Motivation

When a global default dispatcher has already been set, the
`dispatch::set_global_default` function fails with a
`SetGlobalDefaultError`. The `fmt::Display` impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the `fmt::Debug` impl is derived, and just looks
like this:
```
SetGlobalDefaultError { _no_construct: () }
```
which isn't particularly helpful.

Unfortunately, when `unwrap()`ping or `expect()`ing a `Result`, the
`fmt::Debug` implementation for the error type is used, rather than
`fmt::Display`. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.

## Solution

This branch replaces the derived `Debug` impl with a manually
implemented one that prints the same message as the `Display` impl, but
formatted like a tuple struct with a single string field. This avoids
emitting `Debug` output that's *just* a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like
```rust
tracing::dispatch::set_global_default(foo).unwrap();
```

The mesasge now looks like:
```
SetGlobalDefaultError("a global default trace dispatcher has already been set")
```
which should be a bit easier to debug.
@hawkw hawkw requested a review from carllerche as a code owner July 27, 2022 20:43
@hawkw hawkw merged commit f6602ee into master Jul 27, 2022
@hawkw hawkw deleted the eliza/good-global-default-err branch July 27, 2022 21:52
hawkw added a commit that referenced this pull request Jul 29, 2022
## Motivation

When a global default dispatcher has already been set, the
`dispatch::set_global_default` function fails with a
`SetGlobalDefaultError`. The `fmt::Display` impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the `fmt::Debug` impl is derived, and just looks
like this:
```
SetGlobalDefaultError { _no_construct: () }
```
which isn't particularly helpful.

Unfortunately, when `unwrap()`ping or `expect()`ing a `Result`, the
`fmt::Debug` implementation for the error type is used, rather than
`fmt::Display`. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.

## Solution

This branch replaces the derived `Debug` impl with a manually
implemented one that prints the same message as the `Display` impl, but
formatted like a tuple struct with a single string field. This avoids
emitting `Debug` output that's *just* a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like
```rust
tracing::dispatch::set_global_default(foo).unwrap();
```

The mesasge now looks like:
```
SetGlobalDefaultError("a global default trace dispatcher has already been set")
```
which should be a bit easier to debug.
@hawkw hawkw mentioned this pull request Jul 29, 2022
hawkw added a commit that referenced this pull request Jul 29, 2022
## Motivation

When a global default dispatcher has already been set, the
`dispatch::set_global_default` function fails with a
`SetGlobalDefaultError`. The `fmt::Display` impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the `fmt::Debug` impl is derived, and just looks
like this:
```
SetGlobalDefaultError { _no_construct: () }
```
which isn't particularly helpful.

Unfortunately, when `unwrap()`ping or `expect()`ing a `Result`, the
`fmt::Debug` implementation for the error type is used, rather than
`fmt::Display`. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.

## Solution

This branch replaces the derived `Debug` impl with a manually
implemented one that prints the same message as the `Display` impl, but
formatted like a tuple struct with a single string field. This avoids
emitting `Debug` output that's *just* a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like
```rust
tracing::dispatch::set_global_default(foo).unwrap();
```

The mesasge now looks like:
```
SetGlobalDefaultError("a global default trace dispatcher has already been set")
```
which should be a bit easier to debug.
hawkw added a commit that referenced this pull request Jul 29, 2022
## Motivation

When a global default dispatcher has already been set, the
`dispatch::set_global_default` function fails with a
`SetGlobalDefaultError`. The `fmt::Display` impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the `fmt::Debug` impl is derived, and just looks
like this:
```
SetGlobalDefaultError { _no_construct: () }
```
which isn't particularly helpful.

Unfortunately, when `unwrap()`ping or `expect()`ing a `Result`, the
`fmt::Debug` implementation for the error type is used, rather than
`fmt::Display`. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.

## Solution

This branch replaces the derived `Debug` impl with a manually
implemented one that prints the same message as the `Display` impl, but
formatted like a tuple struct with a single string field. This avoids
emitting `Debug` output that's *just* a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like
```rust
tracing::dispatch::set_global_default(foo).unwrap();
```

The mesasge now looks like:
```
SetGlobalDefaultError("a global default trace dispatcher has already been set")
```
which should be a bit easier to debug.
hawkw added a commit that referenced this pull request Jul 29, 2022
# 0.1.29 (July 29, 2022)

This release of `tracing-core` adds `PartialEq` and `Eq` implementations
for metadata types, and improves error messages when setting the global
default subscriber fails.

### Added

- `PartialEq` and `Eq` implementations for `Metadata` ([#2229])
- `PartialEq` and `Eq` implementations for `FieldSet` ([#2229])

### Fixed

- Fixed unhelpful `fmt::Debug` output for
  `dispatcher::SetGlobalDefaultError` ([#2250])
- Fixed compilation with `-Z minimal-versions` ([#2246])

Thanks to @jswrenn and @CAD97 for contributing to this release!

[#2229]: #2229
[#2246]: #2246
[#2250]: #2250
hawkw added a commit that referenced this pull request Jul 29, 2022
# 0.1.29 (July 29, 2022)

This release of `tracing-core` adds `PartialEq` and `Eq` implementations
for metadata types, and improves error messages when setting the global
default subscriber fails.

### Added

- `PartialEq` and `Eq` implementations for `Metadata` ([#2229])
- `PartialEq` and `Eq` implementations for `FieldSet` ([#2229])

### Fixed

- Fixed unhelpful `fmt::Debug` output for
  `dispatcher::SetGlobalDefaultError` ([#2250])
- Fixed compilation with `-Z minimal-versions` ([#2246])

Thanks to @jswrenn and @CAD97 for contributing to this release!

[#2229]: #2229
[#2246]: #2246
[#2250]: #2250
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant