Commit Graph

3 Commits

Author SHA1 Message Date
Ian Manske
62e56d3581
Rework operator type errors (#14429)
# Description

This PR adds two new `ParseError` and `ShellError` cases for type errors
relating to operators.
- `OperatorUnsupportedType` is used when a type is not supported by an
operator in any way, shape, or form. E.g., `+` does not support `bool`.
- `OperatorIncompatibleTypes` is used when a operator is used with types
it supports, but the combination of types provided cannot be used
together. E.g., `filesize + duration` is not a valid combination.

The other preexisting error cases related to operators have been removed
and replaced with the new ones above. Namely:

- `ShellError::OperatorMismatch`
- `ShellError::UnsupportedOperator`
- `ParseError::UnsupportedOperationLHS`
- `ParseError::UnsupportedOperationRHS`
- `ParseError::UnsupportedOperationTernary`

# User-Facing Changes

- `help operators` now lists the precedence of `not` as 55 instead of 0
(above the other boolean operators). Fixes #13675.
- `math median` and `math mode` now ignore NaN values so that `[NaN NaN]
| math median` and `[NaN NaN] | math mode` no longer trigger a type
error. Instead, it's now an empty input error. Fixing this in earnest
can be left for a future PR.
- Comparisons with `nan` now return false instead of causing an error.
E.g., `1 == nan` is now `false`.
- All the operator type errors have been standardized and reworked. In
particular, they can now have a help message, which is currently used
for types errors relating to `++`.

```nu
[1] ++ 2
```
```
Error: nu::parser::operator_unsupported_type

  × The '++' operator does not work on values of type 'int'.
   ╭─[entry #1:1:5]
 1 │ [1] ++ 2
   ·     ─┬ ┬
   ·      │ ╰── int
   ·      ╰── does not support 'int'
   ╰────
  help: if you meant to append a value to a list or a record to a table, use the `append` command or wrap the value in a list. For example: `$list ++ $value` should be
        `$list ++ [$value]` or `$list | append $value`.
```
2025-02-12 20:03:40 -08:00
Wind
2f18b9c856
Enable nushell error with backtrace (#14945)
# Description
After this pr, nushell is able to raise errors with a backtrace, which
should make users easier to debug. To enable the feature, users need to
set env variable via `$env.NU_BACKTRACE = 1`. But yeah it might not work
perfectly, there are some corner cases which might not be handled.

I think it should close #13379 in another way.

### About the change

The implementation mostly contained with 2 parts:
1. introduce a new `ChainedError` struct as well as a new
`ShellError::ChainedError` variant. If `eval_instruction` returned an
error, it converts the error to `ShellError::ChainedError`.
`ChainedError` struct is responsable to display errors properly. It
needs to handle the following 2 cases:
- if we run a function which runs `error make` internally, it needs to
display the error itself along with caller span.
- if we run a `error make` directly, or some commands directly returns
an error, we just want nushell raise an error about `error make`.

2. Attach caller spans to `ListStream` and `ByteStream`, because they
are lazy streams, and *only* contains the span that runs it
directly(like `^false`, for example), so nushell needs to add all caller
spans to the stream.
For example: in `def a [] { ^false }; def b [] { a; 33 }; b`, when we
run `b`, which runs `a`, which runs `^false`, the `ByteStream` only
contains the span of `^false`, we need to make it contains the span of
`a`, so nushell is able to get all spans if something bad happened.
This behavior is happened after running `Instruction::Call`, if it
returns a `ByteStream` and `ListStream`, it will call `push_caller_span`
method to attach call spans.

# User-Facing Changes
It's better to demostrate how it works by examples, given the following
definition:
```nushell
> $env.NU_BACKTRACE = 1
> def a [x] { if $x == 3 { error make {msg: 'a custom error'}}}
> def a_2 [x] { if $x == 3 { ^false } else { $x } }
> def a_3 [x] { if $x == 3 { [1 2 3] | each {error make {msg: 'a custom error inside list stream'} } } }
> def b [--list-stream --external] {
    if $external == true {
        # error with non-zero exit code, which is generated from external command.
        a_2 1; a_2 3; a_2 2
    } else if $list_stream == true {
        # error generated by list-stream
        a_3 1; a_3 3; a_3 2
    } else {
        # error generated by command directly
        a 1; a 2; a 3
    }
}
```

Run `b` directly shows the following error:

<details>

```nushell
Error: chained_error

  × oops
   ╭─[entry #27:1:1]
 1 │ b
   · ┬
   · ╰── error happened when running this
   ╰────

Error: chained_error

  × oops
    ╭─[entry #26:10:19]
  9 │         # error generated by command directly
 10 │         a 1; a 2; a 3
    ·                   ┬
    ·                   ╰── error happened when running this
 11 │     }
    ╰────

Error:
  × a custom error
   ╭─[entry #6:1:26]
 1 │ def a [x] { if $x == 3 { error make {msg: 'a custom error'}}}
   ·                          ─────┬────
   ·                               ╰── originates from here
   ╰────
```

</details>

Run `b --list-stream` shows the following error

<details>

```nushell
Error: chained_error

  × oops
   ╭─[entry #28:1:1]
 1 │ b --list-stream
   · ┬
   · ╰── error happened when running this
   ╰────

Error: nu:🐚:eval_block_with_input

  × Eval block failed with pipeline input
   ╭─[entry #26:7:16]
 6 │         # error generated by list-stream
 7 │         a_3 1; a_3 3; a_3 2
   ·                ─┬─
   ·                 ╰── source value
 8 │     } else {
   ╰────

Error: nu:🐚:eval_block_with_input

  × Eval block failed with pipeline input
   ╭─[entry #23:1:29]
 1 │ def a_3 [x] { if $x == 3 { [1 2 3] | each {error make {msg: 'a custom error inside list stream'} } } }
   ·                             ┬
   ·                             ╰── source value
   ╰────

Error:
  × a custom error inside list stream
   ╭─[entry #23:1:44]
 1 │ def a_3 [x] { if $x == 3 { [1 2 3] | each {error make {msg: 'a custom error inside list stream'} } } }
   ·                                            ─────┬────
   ·                                                 ╰── originates from here
   ╰────
```

</details>

Run `b --external` shows the following error:

<details>

```nushell
Error: chained_error

  × oops
   ╭─[entry #29:1:1]
 1 │ b --external
   · ┬
   · ╰── error happened when running this
   ╰────

Error: nu:🐚:eval_block_with_input

  × Eval block failed with pipeline input
   ╭─[entry #26:4:16]
 3 │         # error with non-zero exit code, which is generated from external command.
 4 │         a_2 1; a_2 3; a_2 2
   ·                ─┬─
   ·                 ╰── source value
 5 │     } else if $list_stream == true {
   ╰────

Error: nu:🐚:non_zero_exit_code

  × External command had a non-zero exit code
   ╭─[entry #7:1:29]
 1 │ def a_2 [x] { if $x == 3 { ^false } else { $x } }
   ·                             ──┬──
   ·                               ╰── exited with code 1
   ╰────
```

</details>

It also added a message to guide the usage of NU_BACKTRACE, see the last
line in the following example:
```shell
 ls asdfasd
Error: nu:🐚:io::not_found

  × I/O error
  ╰─▶   × Entity not found

   ╭─[entry #17:1:4]
 1 │ ls asdfasd
   ·    ───┬───
   ·       ╰── Entity not found
   ╰────
  help: The error occurred at '/home/windsoilder/projects/nushell/asdfasd'

set the `NU_BACKTRACE=1` environment variable to display a backtrace.
```
# Tests + Formatting
Added some tests for the behavior.

# After Submitting
2025-02-06 22:05:58 +08:00
Piepmatz
66bc0542e0
Refactor I/O Errors (#14927)
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

As mentioned in #10698, we have too many `ShellError` variants, with
some even overlapping in meaning. This PR simplifies and improves I/O
error handling by restructuring `ShellError` related to I/O issues.
Previously, `ShellError::IOError` only contained a message string,
making it convenient but overly generic. It was widely used without
providing spans (#4323).

This PR introduces a new `ShellError::Io` variant that consolidates
multiple I/O-related errors (except for `ShellError::NetworkFailure`,
which remains distinct for now). The new `ShellError::Io` variant
replaces the following:

- `FileNotFound`
- `FileNotFoundCustom`
- `IOInterrupted`
- `IOError`
- `IOErrorSpanned`
- `NotADirectory`
- `DirectoryNotFound`
- `MoveNotPossible`
- `CreateNotPossible`
- `ChangeAccessTimeNotPossible`
- `ChangeModifiedTimeNotPossible`
- `RemoveNotPossible`
- `ReadingFile`

## The `IoError`
`IoError` includes the following fields:

1. **`kind`**: Extends `std::io::ErrorKind` to specify the type of I/O
error without needing new `ShellError` variants. This aligns with the
approach used in `std::io::Error`. This adds a second dimension to error
reporting by combining the `kind` field with `ShellError` variants,
making it easier to describe errors in more detail. As proposed by
@kubouch in [#design-discussion on
Discord](https://discord.com/channels/601130461678272522/615329862395101194/1323699197165178930),
this helps reduce the number of `ShellError` variants. In the error
report, the `kind` field is displayed as the "source" of the error,
e.g., "I/O error," followed by the specific kind of I/O error.
2. **`span`**: A non-optional field to encourage providing spans for
better error reporting (#4323).
3. **`path`**: Optional `PathBuf` to give context about the file or
directory involved in the error (#7695). If provided, it’s shown as a
help entry in error reports.
4. **`additional_context`**: Allows adding custom messages when the
span, kind, and path are insufficient. This is rendered in the error
report at the labeled span.
5. **`location`**: Sometimes, I/O errors occur in the engine itself and
are not caused directly by user input. In such cases, if we don’t have a
span and must set it to `Span::unknown()`, we need another way to
reference the error. For this, the `location` field uses the new
`Location` struct, which records the Rust file and line number where the
error occurred. This ensures that we at least know the Rust code
location that failed, helping with debugging. To make this work, a new
`location!` macro was added, which retrieves `file!`, `line!`, and
`column!` values accurately. If `Location::new` is used directly, it
issues a warning to remind developers to use the macro instead, ensuring
consistent and correct usage.

### Constructor Behavior
`IoError` provides five constructor methods:
- `new` and `new_with_additional_context`: Used for errors caused by
user input and require a valid (non-unknown) span to ensure precise
error reporting.
- `new_internal` and `new_internal_with_path`: Used for internal errors
where a span is not available. These methods require additional context
and the `Location` struct to pinpoint the source of the error in the
engine code.
- `factory`: Returns a closure that maps an `std::io::Error` to an
`IoError`. This is useful for handling multiple I/O errors that share
the same span and path, streamlining error handling in such cases.

## New Report Look
This is simulation how the I/O errors look like (the `open crates` is
simulated to show how internal errors are referenced now):
![Screenshot 2025-01-25
190426](https://github.com/user-attachments/assets/a41b6aa6-a440-497d-bbcc-3ac0121c9226)

## `Span::test_data()`
To enable better testing, `Span::test_data()` now returns a value
distinct from `Span::unknown()`. Both `Span::test_data()` and
`Span::unknown()` refer to invalid source code, but having a separate
value for test data helps identify issues during testing while keeping
spans unique.

## Cursed Sneaky Error Transfers
I removed the conversions between `std::io::Error` and `ShellError` as
they often removed important information and were used too broadly to
handle I/O errors. This also removed the problematic implementation
found here:

7ea4895513/crates/nu-protocol/src/errors/shell_error.rs (L1534-L1583)

which hid some downcasting from I/O errors and made it hard to trace
where `ShellError` was converted into `std::io::Error`. To address this,
I introduced a new struct called `ShellErrorBridge`, which explicitly
defines this transfer behavior. With `ShellErrorBridge`, we can now
easily grep the codebase to locate and manage such conversions.

## Miscellaneous
- Removed the OS error added in #14640, as it’s no longer needed.
- Improved error messages in `glob_from` (#14679).
- Trying to open a directory with `open` caused a permissions denied
error (it's just what the OS provides). I added a `is_dir` check to
provide a better error in that case.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

- Error outputs now include more detailed information and are formatted
differently, including updated error codes.
- The structure of `ShellError` has changed, requiring plugin authors
and embedders to update their implementations.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

I updated tests to account for the new I/O error structure and
formatting changes.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

This PR closes #7695 and closes #14892 and partially addresses #4323 and
#10698.

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
2025-01-28 16:03:31 -06:00