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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make "NAR info file is corrupt" messages more informative #8233

Merged
merged 1 commit into from Apr 19, 2023

Conversation

wentasah
Copy link
Contributor

@wentasah wentasah commented Apr 17, 2023

Motivation

Recently, I encountered the NAR info file 'xxxx' is corrupt error with my binary cache. The message is not helpful in determining, which kind of corruption happened. The file, fetched with curl, looked reasonably.

This commit adds more information to the error message, which should allow debugging and hopefully fixing the problem. In my case, there was an extra empty line at the end of the narinfo file so the error reported in my case was: NAR info file 'xxxx' is corrupt: expecting ':' at line 8.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 馃憤 to pull requests you find important.

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks, that could be useful indeed :)

Do you think you could also add some line numbers to the error message (I assume that just increasing a counter at each iteration would be enough)? That way messages like expecting ':' would really be meaningful and point to the right line

Recently, I encountered the "NAR info file 'xxxx' is corrupt" error
with my binary cache. The message is not helpful in determining, which
kind of corruption happened. The file, fetched with curl, looked
reasonably.

This commit adds more information to the error message, which should
allow debugging and hopefully fixing the problem.
@wentasah
Copy link
Contributor Author

Good idea. I've added line numbers and also updated the NarSize missing or zero message to better reflect the situation.

Tested with intentionally broken nix-serve.

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks :)

@thufschmitt thufschmitt merged commit d3e2394 into NixOS:master Apr 19, 2023
10 checks passed
@fricklerhandwerk fricklerhandwerk added the error-messages Confusing messages and better diagnostics label Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Confusing messages and better diagnostics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants