feat(cache): Stuff error messages into Malformed and CacheSpecificError cache files
#510
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Swatinem
approved these changes
Aug 4, 2021
| log::warn!("Error while parsing cficache: {}", LogError(&err)); | ||
| log::warn!( | ||
| "Error while parsing cficache: {} ({})", | ||
| LogError(&err), |
There was a problem hiding this comment.
doesn’t LogError already contain the details?
There was a problem hiding this comment.
unfortunately not: LogError in this case just spits out an in-depth string for CfiCacheError::ObjectParsing(ObjectError::Malformed).
details may contain error info that may have been propagated from the download cache if the download cache itself has a malformed entry for the same file or it could contain error info during a previous attempt to compute the cfi cache entry.
loewenheim
approved these changes
Aug 4, 2021
relaxolotl
force-pushed
the
feat/recognize-extended-cache-statuses
branch
from
e240e55
to
626a5cd
Compare
Base automatically changed from
feat/recognize-extended-cache-statuses
to
master
August 4, 2021 19:29
relaxolotl
changed the title
Malformed and CacheSpecificError cache files
relaxolotl
force-pushed
the
feat/stuff-messages-into-cache
branch
from
8288203
to
7722e79
Compare
relaxolotl
changed the base branch from
master
to
feat/per-cache-expiration-strats
this has zero impact on how malformed entries behave. they only include additional info that can be used for debugging, if needed. the only functional change is that the contents of malformed cache entries may be a little longer, as they'll include an error message.
this has zero impact on the existing behaviour of cache entries. cache-specific error cache entries will have a little bit of extra debugging info if needed during development.
relaxolotl
force-pushed
the
feat/stuff-messages-into-cache
branch
from
7722e79
to
378b8ba
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Welcome to part 2 of my PR series on making symbolicator's cache statuses gooder. Before reviewing or merging this PR, you may be interested in taking a look at part 1 first for some context: #509
If you've already seen #509 or you're brave enough to be look at these changes as-is because you reviewed an eerily similar set of changes in a previous PR, then let's continue.
This belongs to the following chain of PRs:
CacheSpecificError#509)CacheStatus::CacheSpecificErrorentries have different timeout behaviours based on the type of cache they belong to #516)MalformedandCacheSpecificErrorcache files #510) 👈 you are hereThis PR does one thing: it stuffs an error string returned by the
Displayimpl of the error intoMalformedandCacheSpecificErrorentries, and adds all of the plumbing needed to write and read them from their respective cache files. This makes no functional changes to symbolicator.