Skip to content

Panic serving a web request #1272

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

Closed
jyn514 opened this issue Feb 11, 2021 · 5 comments
Closed

Panic serving a web request #1272

jyn514 opened this issue Feb 11, 2021 · 5 comments
Labels
A-backend Area: Webserver backend C-bug Category: This is a bug E-easy Effort: Should be easy to implement and would make a good first PR help-wanted

Comments

@jyn514
Copy link
Member

jyn514 commented Feb 11, 2021

Feb 11 17:03:59 docsrs cratesfyi[20003]: thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', src/storage/s3.rs:122:48
Feb 11 17:03:59 docsrs cratesfyi[20003]: stack backtrace:
Feb 11 17:03:59 docsrs cratesfyi[20003]:    0: backtrace::backtrace::libunwind::trace
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
Feb 11 17:03:59 docsrs cratesfyi[20003]:    1: backtrace::backtrace::trace_unsynchronized
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
Feb 11 17:03:59 docsrs cratesfyi[20003]:    2: std::sys_common::backtrace::_print_fmt
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/sys_common/backtrace.rs:78
Feb 11 17:03:59 docsrs cratesfyi[20003]:    3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/sys_common/backtrace.rs:59
Feb 11 17:03:59 docsrs cratesfyi[20003]:    4: core::fmt::write
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libcore/fmt/mod.rs:1076
Feb 11 17:03:59 docsrs cratesfyi[20003]:    5: std::io::Write::write_fmt
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/io/mod.rs:1537
Feb 11 17:03:59 docsrs cratesfyi[20003]:    6: std::sys_common::backtrace::_print
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/sys_common/backtrace.rs:62
Feb 11 17:03:59 docsrs cratesfyi[20003]:    7: std::sys_common::backtrace::print
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/sys_common/backtrace.rs:49
Feb 11 17:03:59 docsrs cratesfyi[20003]:    8: std::panicking::default_hook::{{closure}}
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/panicking.rs:198
Feb 11 17:03:59 docsrs cratesfyi[20003]:    9: std::panicking::default_hook
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/panicking.rs:218
Feb 11 17:03:59 docsrs cratesfyi[20003]:   10: std::panicking::rust_panic_with_hook
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/panicking.rs:486
Feb 11 17:03:59 docsrs cratesfyi[20003]:   11: rust_begin_unwind
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libstd/panicking.rs:388
Feb 11 17:03:59 docsrs cratesfyi[20003]:   12: core::panicking::panic_fmt
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libcore/panicking.rs:101
Feb 11 17:03:59 docsrs cratesfyi[20003]:   13: core::panicking::panic
Feb 11 17:03:59 docsrs cratesfyi[20003]:              at src/libcore/panicking.rs:56
Feb 11 17:03:59 docsrs cratesfyi[20003]:   14: docs_rs::storage::s3::S3Backend::get::{{closure}}
Feb 11 17:03:59 docsrs cratesfyi[20003]:   15: std::thread::local::LocalKey<T>::with
Feb 11 17:03:59 docsrs cratesfyi[20003]:   16: tokio::runtime::enter::Enter::block_on
Feb 11 17:03:59 docsrs cratesfyi[20003]:   17: tokio::runtime::thread_pool::ThreadPool::block_on
Feb 11 17:03:59 docsrs cratesfyi[20003]:   18: tokio::runtime::Runtime::block_on
Feb 11 17:03:59 docsrs cratesfyi[20003]:   19: docs_rs::storage::Storage::get
Feb 11 17:03:59 docsrs cratesfyi[20003]:   20: <docs_rs::web::file::DatabaseFileHandler as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   21: <alloc::boxed::Box<dyn iron::middleware::Handler> as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   22: <docs_rs::web::metrics::RequestRecorder as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   23: <alloc::boxed::Box<dyn iron::middleware::Handler> as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   24: <docs_rs::web::routes::BlockBlacklistedPrefixes as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   25: <alloc::boxed::Box<dyn iron::middleware::Handler> as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   26: <docs_rs::web::CratesfyiHandler as iron::middleware::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   27: <iron::iron::RawHandler<H> as hyper::server::Handler>::handle
Feb 11 17:03:59 docsrs cratesfyi[20003]:   28: hyper::server::Worker<H>::handle_connection
Feb 11 17:03:59 docsrs cratesfyi[20003]:   29: hyper::server::listener::spawn_with::{{closure}}
Feb 11 17:03:59 docsrs cratesfyi[20003]: note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@jyn514 jyn514 added C-bug Category: This is a bug A-backend Area: Webserver backend labels Feb 11, 2021
@Nemo157
Copy link
Member

Nemo157 commented Feb 11, 2021

Panicking line:

let date_updated = parse_timespec(&res.last_modified.unwrap())?;

Seems like S3 returned something without a Last-Modified header.

@syphar
Copy link
Member

syphar commented Feb 13, 2021

@Nemo157 @jyn514 I think this value is only used to set the Last-Modified header when serving these files directly, right?

IMHO then just using the current date/time as default would suffice, would it?

@jyn514
Copy link
Member Author

jyn514 commented Feb 13, 2021

Sure, that seems reasonable.

syphar added a commit to syphar/docs.rs that referenced this issue Feb 14, 2021
syphar added a commit to syphar/docs.rs that referenced this issue Feb 14, 2021
@jyn514 jyn514 added E-easy Effort: Should be easy to implement and would make a good first PR help-wanted labels Feb 15, 2021
@syphar
Copy link
Member

syphar commented Feb 15, 2021

Hm... while the fix itself is easy (syphar@e1fa704 ) ,
I'm not sure about two things:

  1. I didn't find a quick way to get minio to return an empty Last-Modified header. So writing tests would (if I'm not mistaken) invoke refactoring the s3-storage to support us providing a mocked client via rusoto_mock, or mocking the http responses with mockito, for the whole auth+read process. And I'm not sure it is worth the effort.
  2. since, by documentation, this field should be always filled with a value, at least the creation-date of the object. So this might by just a symptom of a bigger issue with S3, or rusoto.

I personally tend to say we take this quickfix and watch it, because the default is fine.

But up to you :)

@jyn514
Copy link
Member Author

jyn514 commented Feb 15, 2021

Personally, I'm fine with fixing this without a test.

@jyn514 jyn514 closed this as completed in e1fa704 Feb 16, 2021
jyn514 added a commit that referenced this issue Feb 16, 2021
Fixes #1272 - handle S3 not returning last-modified in some cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend Area: Webserver backend C-bug Category: This is a bug E-easy Effort: Should be easy to implement and would make a good first PR help-wanted
Projects
None yet
Development

No branches or pull requests

3 participants