Issue 56: Remove try/catch from skill module directory scanning
Replace the try/catch block in collect_paths() lambda with explicit std::error_code-based error handling using recursive_directory_iterator constructor and increment() methods that don't throw exceptions. Error messages include the path that failed and the underlying error message from the filesystem operation. Added test_skill_unreadable_dir.sh to verify proper error handling when a directory is unreadable.
This commit is contained in:
+17
-7
@@ -127,14 +127,24 @@ std::expected<std::vector<SkillMetadata>, std::string> scan_skills() {
|
||||
if (!fs::exists(path) || !fs::is_directory(path)) {
|
||||
return {}; // Silently skip non-existent directories
|
||||
}
|
||||
try {
|
||||
for (const auto& entry : fs::recursive_directory_iterator(path)) {
|
||||
if (entry.is_regular_file() && entry.path().filename() == "SKILL.md") {
|
||||
discovered_paths.push_back(entry.path().string());
|
||||
}
|
||||
|
||||
std::error_code ec;
|
||||
auto it = fs::recursive_directory_iterator(path, ec);
|
||||
if (ec) {
|
||||
return std::unexpected("Could not scan directory " + path + ": " + ec.message());
|
||||
}
|
||||
|
||||
auto end = fs::recursive_directory_iterator();
|
||||
|
||||
while (it != end) {
|
||||
if (it->is_regular_file() && it->path().filename() == "SKILL.md") {
|
||||
discovered_paths.push_back(it->path().string());
|
||||
}
|
||||
|
||||
it.increment(ec);
|
||||
if (ec) {
|
||||
return std::unexpected("Could not scan directory " + path + ": " + ec.message());
|
||||
}
|
||||
} catch (const fs::filesystem_error& e) {
|
||||
return std::unexpected(std::string("Could not scan directory ") + path + ": " + e.what());
|
||||
}
|
||||
return {};
|
||||
};
|
||||
|
||||
@@ -37,6 +37,7 @@ add_dr_test(test_editor)
|
||||
add_dr_test(test_tool_skill)
|
||||
add_dr_test(test_skill_filter)
|
||||
add_dr_test(test_skill_override)
|
||||
add_dr_test(test_skill_unreadable_dir)
|
||||
add_dr_test(test_auto_temp_file)
|
||||
|
||||
# Add JSON parsing and error handling tests
|
||||
|
||||
Executable
+41
@@ -0,0 +1,41 @@
|
||||
#!/bin/bash
|
||||
# test_skill_unreadable_dir.sh - Verifies that unreadable directories produce proper error messages
|
||||
|
||||
# Create a skill directory with proper content
|
||||
mkdir -p skills
|
||||
cat > skills/SKILL.md << 'SKILL'
|
||||
---
|
||||
name: test-skill
|
||||
---
|
||||
# Test Skill
|
||||
SKILL
|
||||
|
||||
# Create a nested directory and make it unreadable
|
||||
mkdir -p skills/unreadable_subdir
|
||||
chmod 000 skills/unreadable_subdir
|
||||
|
||||
# Run drharness with -D flag and capture stderr
|
||||
set +e
|
||||
OUTPUT=$("${DRHARNESS}" -D -F dummy.chat 2>&1)
|
||||
EXIT_CODE=$?
|
||||
set -e
|
||||
|
||||
# Verify that it exits with non-zero code due to permission error
|
||||
if [ "$EXIT_CODE" -eq 0 ]; then
|
||||
echo "FAIL: Expected non-zero exit code for unreadable directory, got 0"
|
||||
chmod 755 skills/unreadable_subdir
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Verify the error message mentions the directory path
|
||||
if ! echo "$OUTPUT" | grep -q "Could not scan directory"; then
|
||||
echo "FAIL: Expected 'Could not scan directory' in error output"
|
||||
echo "Got: $OUTPUT"
|
||||
chmod 755 skills/unreadable_subdir
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Clean up (restore permissions before removing)
|
||||
chmod 755 skills/unreadable_subdir
|
||||
|
||||
echo "PASS: test_skill_unreadable_dir"
|
||||
Reference in New Issue
Block a user