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

More informative error message in case of corrupted blob headers #759

Conversation

Andrius-B
Copy link
Contributor

I would love to get a more informative error message here. I keep seeing this happen but since the error does not contain the hash of the CAS item being fetched, it's very tricky to figure out what goes wrong from the access logs.
Specifically the error I'm seeing is this:

Warning: expected item to be on disk, but something happened: offset table values should increase: 0 -> 29

I'm hoping that with a hash to reference in the logs, I can find out what corrupts the headers

@@ -476,7 +476,7 @@ func (c *diskCache) availableOrTryProxy(kind cache.EntryKind, hash string, size
}

if err != nil {
log.Printf("Warning: expected item to be on disk, but something happened: %v", err)
log.Printf("Warning: expected item to be on disk, but something happened when retrieving %s-%d: %v", hash, size, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution.

I'm happy to improve the logging here, but perhaps we can improve this a bit, eg:

  • It might be worth logging blobPath here instead, like the log in the if case one level up.
  • Maybe we could also log something different in each of the cases in the if-else block immediately before this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi! I have adjusted the error as suggested and added some additional variables that should make it easy to figure out which kind of ReadCloser is being fetched.
In the end my problem was that I was running the program with too little resources and that caused it to fail in various interesting ways. Once I gave it some more CPUs, it started running like expected. Although this particular error message was not very informative so it would be cool to make it better

@Andrius-B Andrius-B force-pushed the better-error-message-in-case-of-corrupted-headers branch from 27bfb8f to 2a4cffe Compare August 7, 2024 20:43
@Andrius-B Andrius-B force-pushed the better-error-message-in-case-of-corrupted-headers branch from 2a4cffe to e626f66 Compare August 7, 2024 20:46
@mostynb
Copy link
Collaborator

mostynb commented Aug 12, 2024

Thanks for the contribution- this has landed on the master branch.

@mostynb mostynb closed this Aug 12, 2024
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.

None yet

2 participants