nushell/crates/nu-command/tests/commands/network/http/delete.rs

146 lines
3.2 KiB
Rust
Raw Permalink Normal View History

use std::{thread, time::Duration};
use mockito::Server;
use nu_test_support::{nu, pipeline};
#[test]
fn http_delete_is_success() {
let mut server = Server::new();
let _mock = server.mock("DELETE", "/").create();
let actual = nu!(pipeline(
format!(
r#"
http delete {url}
"#,
url = server.url()
)
.as_str()
));
assert!(actual.out.is_empty())
}
#[test]
fn http_delete_is_success_pipeline() {
let mut server = Server::new();
let _mock = server.mock("DELETE", "/").create();
let actual = nu!(pipeline(
format!(
r#"
"foo" | http delete {url}
"#,
url = server.url()
)
.as_str()
));
assert!(actual.out.is_empty())
}
#[test]
fn http_delete_failed_due_to_server_error() {
let mut server = Server::new();
changes Reqwest to Ureq. (#8320) # Description _(Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes.)_ This pull request removes `Reqwest` and replaces it with `Ureq` to remove some of our dependencies, giving us faster compile times as well as smaller binaries. `Ureq` does not have an async runtime included so we do not need build heavy dependencies such as `Tokio`. From older tests I had the number of build units be reduced from `430 -> 392`. The default of `Ureq` uses `Rustls` but it has been configured to instead use `native_tls` which should work exactly the same as the `tls` works now. I removed `content-length` from the http commands as after refactoring i did not see a reason to have it available, correct me if this is something we should preserve. In the medium, to long term, we should maybe consider changing to `rustls` to have the same `tls` on all platforms. # User-Facing Changes _(List of all changes that impact the user experience here. This helps us keep track of breaking changes.)_ # 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 -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # 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.
2023-03-05 23:48:13 +01:00
let _mock = server.mock("DELETE", "/").with_status(400).create();
let actual = nu!(pipeline(
format!(
r#"
http delete {url}
"#,
url = server.url()
)
.as_str()
));
assert!(actual.err.contains("Bad request (400)"))
}
Allow `http` commands' automatic redirect-following to be disabled (#11329) Intends to close #8920 This PR suggests a new flag for the `http` commands, `--redirect-mode`, which enables users to choose between different redirect handling modes. The current behaviour of letting ureq silently follow redirects remains the default, but two new options are introduced here, following the lead of [JavaScript's `fetch` API](https://developer.mozilla.org/en-US/docs/Web/API/fetch#redirect): "manual", where any 3xx response to a request is simply returned as the command's result, and "error", where any 3xx response causes a network error like those caused by 4xx and 5xx responses. This PR is a draft. Tests have not been added or run, the flag is currently only implemented for the `http get` command, and design tweaks are likely to be appropriate. Most notably, it's not obvious to me whether a single flag which can take one of three values is the nicest solution here. We might instead consider two binary flags (like `--no-following-redirects` and `--disallow-redirects`, although I'm bad at naming things so I need help with that anyway), or completely drop the "error" option if it's not deemed useful enough. (I personally think it has some merit, especially since 4xx and 5xx responses are already treated as errors by default; So this would allow users to treat only immediate 2xx responses as success) # User-facing changes New options for the `http [method]` commands. Behaviour remains unchanged when the command line flag introduced here is not used. ![image](https://github.com/nushell/nushell/assets/12228688/1eb89f14-7d48-4f41-8a3e-cc0f1bd0a4f8)
2023-12-28 08:26:34 +01:00
#[test]
fn http_delete_follows_redirect() {
let mut server = Server::new();
let _mock = server.mock("GET", "/bar").with_body("bar").create();
let _mock = server
.mock("DELETE", "/foo")
.with_status(301)
.with_header("Location", "/bar")
.create();
let actual = nu!(pipeline(
format!("http delete {url}/foo", url = server.url()).as_str()
));
assert_eq!(&actual.out, "bar");
}
#[test]
fn http_delete_redirect_mode_manual() {
let mut server = Server::new();
let _mock = server
.mock("DELETE", "/foo")
.with_status(301)
.with_body("foo")
.with_header("Location", "/bar")
.create();
let actual = nu!(pipeline(
format!(
"http delete --redirect-mode manual {url}/foo",
url = server.url()
)
.as_str()
));
assert_eq!(&actual.out, "foo");
}
#[test]
fn http_delete_redirect_mode_error() {
let mut server = Server::new();
let _mock = server
.mock("DELETE", "/foo")
.with_status(301)
.with_body("foo")
.with_header("Location", "/bar")
.create();
let actual = nu!(pipeline(
format!(
"http delete --redirect-mode error {url}/foo",
url = server.url()
)
.as_str()
));
assert!(&actual.err.contains("nu::shell::network_failure"));
assert!(&actual.err.contains(
"Redirect encountered when redirect handling mode was 'error' (301 Moved Permanently)"
));
}
#[test]
fn http_delete_timeout() {
let mut server = Server::new();
let _mock = server
.mock("DELETE", "/")
.with_chunked_body(|w| {
thread::sleep(Duration::from_secs(10));
w.write_all(b"Delayed response!")
})
.create();
let actual = nu!(pipeline(
format!("http delete --max-time 100ms {url}", url = server.url()).as_str()
));
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: https://github.com/nushell/nushell/blob/7ea489551315a5ab68fd1e9e5a7ed5a0b3ef494a/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 23:03:31 +01:00
assert!(&actual.err.contains("nu::shell::io::timed_out"));
assert!(&actual.err.contains("Timed out"));
}