Currently the out of range handling for the SystemTime either panics or returns an Err.
let dur = self
.0
.duration_since(UNIX_EPOCH)
.expect("all times should be after the epoch");
let secs_since_epoch = dur.as_secs();
let nanos = dur.subsec_nanos();
if secs_since_epoch >= 253_402_300_800 {
// year 9999
return Err(fmt::Error);
}
The docs for the Display trait specify that "[fmt] should return Err if, and only if, the provided Formatter returns Err. String formatting is considered an infallible operation...", and for fmt::error itself "This type does not support transmission of an error other than that an error occurred. This is because, despite the existence of this error, string formatting is considered an infallible operation. fmt() implementers should not return this Error unless they received it from their Formatter".
Returning an error isn't very useful for the user here since mostly they would be used in functions like to_string(), format!(), println!(), etc; these all do panic if the result is not Ok, so you cannot see the Err unless you write your own writer, and even in that case you can't differentiate between the formatter failing and the time being out of range. And panicking on duration_since failing isn't really something a user can deal with either.
The more "correct" way to do this would be to have the error checking be inside the format_rfc3339* functions and have these functions return the Result, and then Display::fmt will have no failure cases except from the formatter itself failing. This would be a breaking change so the alternative would be to add new format_rfc3339* methods (i.e. the try_format_.. methods suggested in #68) which do bounds checking and return a Result, which would guarantee fmt can't return Err or panic if using on an Rfc3339Timestamp created that way. I would argue that it doesn't really make sense for a Rfc3339Timestamp to be able to be in a state where it cannot be formatted, so I favour the breaking change, but I understand not wanting to make a breaking change for a rare issue.
Obviously these out of range errors are extremely rare so it's not a high priority issue either way. Even if you choose to keep the bounds checking inside fmt I think the error cases should be consistent, they should both panic or both return Err, rather than one panicking and one returning Err. Again, fmt::Error is not intended for this situation so I would probably avoid that so them both panicking would be my suggestion, especially since there's not much functional difference in returning an Err and panicking in the majority of cases where this function is used anyway.
Related issues: #68
Currently the out of range handling for the SystemTime either panics or returns an Err.
The docs for the Display trait specify that "[fmt] should return Err if, and only if, the provided Formatter returns Err. String formatting is considered an infallible operation...", and for fmt::error itself "This type does not support transmission of an error other than that an error occurred. This is because, despite the existence of this error, string formatting is considered an infallible operation. fmt() implementers should not return this Error unless they received it from their Formatter".
Returning an error isn't very useful for the user here since mostly they would be used in functions like
to_string(),format!(),println!(), etc; these all do panic if the result is not Ok, so you cannot see the Err unless you write your own writer, and even in that case you can't differentiate between the formatter failing and the time being out of range. And panicking onduration_sincefailing isn't really something a user can deal with either.The more "correct" way to do this would be to have the error checking be inside the format_rfc3339* functions and have these functions return the
Result, and thenDisplay::fmtwill have no failure cases except from the formatter itself failing. This would be a breaking change so the alternative would be to add new format_rfc3339* methods (i.e. the try_format_.. methods suggested in #68) which do bounds checking and return aResult, which would guaranteefmtcan't return Err or panic if using on anRfc3339Timestampcreated that way. I would argue that it doesn't really make sense for a Rfc3339Timestamp to be able to be in a state where it cannot be formatted, so I favour the breaking change, but I understand not wanting to make a breaking change for a rare issue.Obviously these out of range errors are extremely rare so it's not a high priority issue either way. Even if you choose to keep the bounds checking inside fmt I think the error cases should be consistent, they should both panic or both return
Err, rather than one panicking and one returningErr. Again,fmt::Erroris not intended for this situation so I would probably avoid that so them both panicking would be my suggestion, especially since there's not much functional difference in returning anErrand panicking in the majority of cases where this function is used anyway.Related issues: #68