Skip to content
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

feat(cache): Infrastructure to distinguish between errors caused by malformed objects vs cache-specific errors #511

Merged
merged 8 commits into from
Aug 17, 2021

Conversation

relaxolotl
Copy link
Contributor

@relaxolotl relaxolotl commented Jul 30, 2021

This is part 3 of a series of PRs focused on cache statuses. Like the others, if you haven't taken a look at the previous PRs and have no context on these changes, it may be helpful to take a look at those first: #509 #510

  1. Introduce the new cache status without changing the existing functionality, convert the new status to Malformed if needed (feat(cache): Add a new cache status CacheSpecificError #509)
  2. Update cache expiration strategy selection so different caches will expire entries triggered by cache-specific problems after a timeout that makes sense for the cache (CacheStatus::CacheSpecificError entries have different timeout behaviours based on the type of cache they belong to #516)
  3. Extend both the new cache status and the Malformed cache status to accept an arbitrary string that describes the root cause of the issue (feat(cache): Stuff error messages into Malformed and CacheSpecificError cache files #510)
  4. 👉 Identify and split out code paths that should be using the new cache status without changing their return value (feat(cache): Infrastructure to distinguish between errors caused by malformed objects vs cache-specific errors #511)👈 you are here
  5. Begin actually using the new cache status by returning it in places identified in the previous step and writing + reading those to and from the cache (feat(cache): Use the CacheSpecificError cache status #512)

This PR is focused on the downloaders. It adds documentation and extends them as needed so it's easy to (eventually) tell the difference between a malformed object and a cache-specific error. This PR makes no functional changes to symbolicator, and simply makes some changes in preparation for step 4.

#skip-changelog

@relaxolotl relaxolotl force-pushed the feat/stuff-messages-into-cache branch from 8288203 to 7722e79 Compare August 4, 2021 21:20
@relaxolotl relaxolotl changed the title feat(cache): Infrastructure to distinguish between errors caused by malformed objects vs download errors feat(cache): Infrastructure to distinguish between errors caused by malformed objects vs cache-specific errors Aug 4, 2021
@relaxolotl relaxolotl force-pushed the feat/stuff-messages-into-cache branch from 7722e79 to 378b8ba Compare August 5, 2021 15:02
Base automatically changed from feat/stuff-messages-into-cache to master August 5, 2021 16:31
log::debug!("Fetched debug file from {:?}: {:?}", source, status);
match status {
DownloadStatus::Completed => {
log::debug!("Fetched debug file from {:?}: {:?}", source, status);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to do that in this PR, but the debug logs here print all of the source information, including private S3 keys, etc to the console, so that is waaay to detailed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll create a follow-up PR that tries to address this in a simple way 👍

@relaxolotl relaxolotl enabled auto-merge (squash) August 6, 2021 17:34
@relaxolotl relaxolotl merged commit 745b89e into master Aug 17, 2021
@relaxolotl relaxolotl deleted the feat/error-granularity branch August 17, 2021 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants