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

invalid date crashes nushell #11710

Closed
kaibaesler opened this issue Feb 2, 2024 · 7 comments
Closed

invalid date crashes nushell #11710

kaibaesler opened this issue Feb 2, 2024 · 7 comments
Labels
needs-triage An issue that hasn't had any proper look panic upstream problem with upstream dependency
Milestone

Comments

@kaibaesler
Copy link

Describe the bug

Feeding into datetime with expressions that have a month-part greater than 12 crashes nu.

How to reproduce

enter "2023-13-31" | into datetime

Results in:

thread 'main' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/human-date-parser-0.1.1/src/lib.rs:186:64:
called `Result::unwrap()` on an `Err` value: ParseError(OutOfRange)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

and Nu exits

Expected behavior

I expect Nu to catch this invalid input and show an error message, like eg. when doing ~> "2023-foo-31" | into datetime:

Error: nu::shell::datetime_parse_error

Screenshots

No response

Configuration

Tested on Linux/Intel (Ubuntu 23.10) installed from nu-0.89.0-x86_64-linux-gnu-full.tar.gz Github release.

key value
version 0.89.0
branch
commit_hash 2c1560e
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.73.0 (cc66ad468 2023-10-03)
rust_channel 1.73.0-x86_64-unknown-linux-gnu
cargo_version cargo 1.73.0 (9c4383fb5 2023-08-26)
build_time 2024-01-09 20:22:14 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, extra, sqlite, static-link-openssl, trash, which, zip
installed_plugins

and macOS/Apple Silicone (Sonoma 14.3) installed with Homebrew

key value
version 0.89.0
branch
commit_hash
build_os macos-aarch64
build_target aarch64-apple-darwin
rust_version rustc 1.75.0 (82e1608df 2023-12-21) (Homebrew)
cargo_version cargo 1.75.0
build_time 2024-01-09 20:16:29 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, extra, sqlite, trash, which, zip
installed_plugins

Additional context

No response

@kaibaesler kaibaesler added the needs-triage An issue that hasn't had any proper look label Feb 2, 2024
@fdncred
Copy link
Collaborator

fdncred commented Feb 2, 2024

This is upstream https://github.com/technologicalMayhem/human-date-parser/blob/a87d39c963fadbef1a9fbb06214497d83420d564/src/lib.rs#L186 where they're doing an unwrap() and shouldn't. Not sure what to do about it here.

@kaibaesler
Copy link
Author

It's not a big deal, as far as I'm concerned. I was just playing around with Nushell, and since it's a panic, I wanted to let you know about it.

And, I've just found out that you guys already implemented seq date, which does exactly what I wanted to achieve with a self-written for-loop and a little bit of date arithmetic.

Keep up the great work!

@kubouch kubouch added this to the 1.0 milestone Feb 2, 2024
@kubouch kubouch added upstream problem with upstream dependency panic labels Feb 2, 2024
@kubouch
Copy link
Contributor

kubouch commented Feb 3, 2024

The issue was already reported upstream technologicalMayhem/human-date-parser#1. There are no replies and the crate seems unmaintained, so we can't hope for an upstream fix. We'd need to either remove the functionality, check if some other maintained crate could be used, or roll our own.

@KAAtheWiseGit
Copy link
Contributor

I'd argue for removing human-readable dates interpretation. I feel like it's better suited for a plugin, not the core of the language.

@fdncred
Copy link
Collaborator

fdncred commented Feb 5, 2024

I feel like I'd be sad to see it go. I kind of like nushell-y-ness of 'In 5 minutes and 30 seconds' | into datetime

@technologicalMayhem
Copy link

I fixed the issue in my code. Sorry for only getting around to it now.

@fdncred
Copy link
Collaborator

fdncred commented Apr 25, 2024

i believe this was fixed recently, please ping me if it's not fixed on the latest main branch and i'll reopen.

@hustcer hustcer modified the milestones: 1.0, v0.93.0 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage An issue that hasn't had any proper look panic upstream problem with upstream dependency
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants