mirror of
https://github.com/discourse/discourse.git
synced 2024-12-03 14:53:38 +08:00
aaec964547
### Background Several call sites use `FileStore#download` (through `Discourse.store.download`). In some cases the author seems aware that the method can raise an error if the download fails, and in some cases not. Because of this we're seeing some of these exceptions bubble all the way up and getting logged in production. Although they are not really actionable at that point. Rather each call site needs to be considered to figure out how to handle them. ### What is this change? This change accomplishes primarily two things. Firstly it separates the method into a safe version which will handle errors by returning `nil`, and an unsafe version which will re-package upstream errors in a new `FileStore::DownloadError` class. Secondly it updates the call sites which have been doing error handling downstream to use the new safe version. For backwards compatibility, there's an interim situation and a desired end state. **Interim:** ``` FileStore#download → Old unsafe version. Will raise any error and show a deprecation warning. FileStore#download! → New unsafe version. Will raise FileStore::DownloadError. FileStore#download_safe → New safe version. Will return nil. ``` **Desired end-state:** ``` FileStore#download → New safe version. Will return nil. FileStore#download! → New unsafe version. Will raise FileStore::DownloadError. ``` ### What's next? We need to do a quick audit of the call sites that are using the old unsafe version without any error handling, as well as check for call sites in plugins other repos. Follow-up PRs incoming. |
||
|---|---|---|
| .. | ||
| base_store_spec.rb | ||
| local_store_spec.rb | ||
| s3_store_spec.rb | ||