Skip to content

BinaryCacheStore::queryPathInfoUncached(): Ensure noexcept#336

Merged
edolstra merged 1 commit into
mainfrom
eelcodolstra/nix-286-uncaught-exception-in-binarycachestorequerypathinfouncached
Feb 5, 2026
Merged

BinaryCacheStore::queryPathInfoUncached(): Ensure noexcept#336
edolstra merged 1 commit into
mainfrom
eelcodolstra/nix-286-uncaught-exception-in-binarycachestorequerypathinfouncached

Conversation

@edolstra
Copy link
Copy Markdown
Collaborator

@edolstra edolstra commented Feb 5, 2026

Motivation

Make sure we don't throw an exception, since that will terminate the program.

Context

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling for binary cache operations, ensuring exceptions are properly caught and handled through a more robust control flow.

Make sure we don't throw an exception, since that will terminate the
program.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

The uncached path query logic in the binary cache store is refactored to wrap its operation in a try-catch block. Logging activity creation and nar-info file lookup are moved inside the try block, with exceptions rethrown via callback in the catch block, adding explicit exception-safety guarantees.

Changes

Cohort / File(s) Summary
Exception-Safety Refactoring
src/libstore/binary-cache-store.cc
Wrapped uncached path query logic in try-catch block; moved logging activity and nar-info lookup into try block; exceptions rethrown through callback in catch handler. No public API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • cole-h

Poem

🐰 A try, a catch, a callback's grace,
Exception safety finds its place,
Where errors hop and get retold,
Through safer paths, both brave and bold!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding exception safety (noexcept guarantee) to the queryPathInfoUncached() function by wrapping logic in try-catch.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch eelcodolstra/nix-286-uncaught-exception-in-binarycachestorequerypathinfouncached

Comment @coderabbitai help to get the list of available commands and usage tips.

@edolstra edolstra enabled auto-merge February 5, 2026 16:05
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 5, 2026

@github-actions github-actions Bot temporarily deployed to pull request February 5, 2026 16:09 Inactive
@edolstra edolstra added this pull request to the merge queue Feb 5, 2026
Merged via the queue into main with commit 5bee101 Feb 5, 2026
28 checks passed
@edolstra edolstra deleted the eelcodolstra/nix-286-uncaught-exception-in-binarycachestorequerypathinfouncached branch February 5, 2026 16:58
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.

2 participants