-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
date: report errors while reading date files #14783
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,3 +108,5 @@ date-error-format-modifier-width-too-large = format modifier width '{$width}' is | |
| date-error-format-missing-plus = the argument {$arg} lacks a leading '+'; | ||
| when using an option to specify date(s), any non-option | ||
| argument must be a format string beginning with '+' | ||
|
|
||
| date-error-read = {$path}: read error: {$error} | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the read-error key to fr-FR.ftl in ee0e92c. Both date locale bundles now define it. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,12 @@ enum DateError { | |
|
|
||
| impl UError for DateError {} | ||
|
|
||
| #[derive(Debug)] | ||
| enum DateInputError { | ||
| InvalidDate(String), | ||
| Read(std::io::Error), | ||
| } | ||
|
|
||
| /// Settings for this program, parsed from the command line | ||
| struct Settings { | ||
| utc: bool, | ||
|
|
@@ -573,7 +579,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { | |
| parse(input, true) | ||
| }; | ||
|
|
||
| let iter = std::iter::once(date); | ||
| let iter = | ||
| std::iter::once(date.map_err(|(input, _)| DateInputError::InvalidDate(input))); | ||
| Box::new(iter) | ||
| } | ||
| DateSource::Stdin => parse_dates_from_reader( | ||
|
|
@@ -672,13 +679,24 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { | |
| } | ||
| } | ||
| } | ||
| Err((input, _err)) => { | ||
| Err(DateInputError::InvalidDate(input)) => { | ||
| let _ = stdout.flush(); | ||
|
|
||
| show!(DateError::InvalidDate { | ||
| date: input.clone() | ||
|
Copilot marked this conversation as resolved.
Outdated
|
||
| }); | ||
| } | ||
| Err(DateInputError::Read(error)) => { | ||
| stdout.flush().map_err(DateError::Write)?; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just above we do
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made this consistent with the invalid-date arm by using let _ = stdout.flush() in f4a7f1d. |
||
| let path = match &settings.date_source { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you please carry the path in DateInputError::Read? re-deriving it from date_source with a hardcoded "-" fallback is fragile
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DateInputError::Read now carries the input path, including the explicit stdin name, in f4a7f1d. |
||
| DateSource::File(path) => path.as_os_str().maybe_quote().to_string(), | ||
| _ => "-".to_string(), | ||
| }; | ||
| return Err(uucore::error::USimpleError::new( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a Read variant to DateError instead of USimpleError here
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added DateError::Read and removed the USimpleError construction in f4a7f1d. |
||
| 1, | ||
| translate!("date-error-read", "path" => path, "error" => strip_errno(&error)), | ||
| )); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1245,24 +1263,31 @@ fn parse_dates_from_reader<R: Read + 'static>( | |
| now: &Zoned, | ||
| dbg_opts: DebugOptions, | ||
| allow_extended: bool, | ||
| ) -> Box< | ||
| dyn Iterator<Item = Result<ParsedDateTime, (String, parse_datetime::ParseDateTimeError)>> + '_, | ||
| > { | ||
| let lines = BufReader::new(reader).split(b'\n'); | ||
| Box::new(lines.map_while(Result::ok).map(move |mut bytes| { | ||
| ) -> Box<dyn Iterator<Item = Result<ParsedDateTime, DateInputError>> + '_> { | ||
| let mut lines = BufReader::new(reader).split(b'\n'); | ||
| let mut failed = false; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the manual
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced the manual flag and from_fn with a mapped iterator plus scan that yields the first read error and then stops in f4a7f1d. |
||
| Box::new(std::iter::from_fn(move || { | ||
| if failed { | ||
| return None; | ||
| } | ||
| let mut bytes = match lines.next()? { | ||
| Ok(bytes) => bytes, | ||
| Err(error) => { | ||
| failed = true; | ||
| return Some(Err(DateInputError::Read(error))); | ||
| } | ||
| }; | ||
| // Strip a trailing '\r' (CRLF input; GNU's lexer ignores it too) | ||
| if bytes.last() == Some(&b'\r') { | ||
| bytes.pop(); | ||
| } | ||
| match String::from_utf8(bytes) { | ||
| Ok(s) => parse_date(s, now, dbg_opts, allow_extended), | ||
| // Report lines with invalid UTF-8 (with non-printable bytes | ||
| // octal-escaped like GNU) instead of silently stopping the input | ||
| Err(e) => Err(( | ||
| escape_invalid_bytes(e.as_bytes()), | ||
| parse_datetime::ParseDateTimeError::InvalidInput, | ||
| )), | ||
| } | ||
| Some(match String::from_utf8(bytes) { | ||
| Ok(s) => parse_date(s, now, dbg_opts, allow_extended) | ||
| .map_err(|(input, _)| DateInputError::InvalidDate(input)), | ||
| Err(e) => Err(DateInputError::InvalidDate(escape_invalid_bytes( | ||
| e.as_bytes(), | ||
| ))), | ||
| }) | ||
| })) | ||
| } | ||
|
|
||
|
|
@@ -1457,6 +1482,32 @@ fn set_system_datetime(date: Zoned) -> UResult<()> { | |
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_reader_reports_error_after_complete_lines() { | ||
| struct FailingReader; | ||
| impl Read for FailingReader { | ||
| fn read(&mut self, _buf: &mut [u8]) -> std::io::Result<usize> { | ||
| Err(std::io::Error::other("read failed")) | ||
| } | ||
| } | ||
|
|
||
| let reader = std::io::Cursor::new(b"@0\r\ninvalid\n@1\n").chain(FailingReader); | ||
| let now = Timestamp::UNIX_EPOCH.to_zoned(TimeZone::UTC); | ||
| let results: Vec<_> = | ||
| parse_dates_from_reader(reader, &now, DebugOptions::new(false, false), false) | ||
| .take(5) | ||
| .collect(); | ||
| assert_eq!(results.len(), 4); | ||
| assert!(results[0].is_ok()); | ||
| assert!( | ||
| matches!(&results[1], Err(DateInputError::InvalidDate(input)) if input == "invalid") | ||
| ); | ||
| assert!(results[2].is_ok()); | ||
| assert!( | ||
| matches!(&results[3], Err(DateInputError::Read(error)) if error.to_string() == "read failed") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_military_timezone_with_offset() { | ||
| // Valid cases: letter only, letter + digit, uppercase | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you check the exact GNU wording/exit code with LANG=C?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. With LANG=C, GNU coreutils 9.12 exits 1, writes no stdout, and reports gnudate: /proc/self/mem: read error: Input/output error on this glibc host. The uutils result matches after normalizing the executable name. The test keeps the OS error suffix flexible because musl spells it I/O error.