Skip to content

Fix NetTrace output path resolution and fail-fast validation for record-trace#272

Merged
brianrob merged 5 commits into
mainfrom
copilot/fix-os-error-2-in-trace-writing
May 14, 2026
Merged

Fix NetTrace output path resolution and fail-fast validation for record-trace#272
brianrob merged 5 commits into
mainfrom
copilot/fix-os-error-2-in-trace-writing

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 14, 2026 •
edited
Loading

Bug Fix

record-trace --on-cpu could pass a directory path directly to NetTrace export when --out was omitted (or pointed to a directory), causing Is a directory (os error 21) at export time. The fix ensures NetTrace always resolves a writable file path up front and fails before collection if output is invalid.

What was the bug?

  • NetTrace validation used the raw output path and only conditionally appended a default filename.
  • Directory-like outputs could reach export as directories instead of files.
  • Path usability (parent existence/type, writable/createable target) was not fully pre-validated before collection began.

How did you fix it?

  • Path resolution
    • Added deterministic NetTrace path resolution:
      • if output is a directory → append trace.nettrace
      • else → treat output as file path
  • Fail-fast output validation
    • Added pre-checks for:
      • output path is not a directory
      • parent path exists and is a directory
      • target file is writable (existing) or createable (new)
    • Validation now returns early errors before recording starts.
  • Regression coverage
    • Added focused unit tests for:
      • directory output with extension still resolves to trace.nettrace
      • missing parent path fails validation early

Example

// NetTrace output path handling
let resolved = if output_path.exists() && output_path.is_dir() {
    output_path.join("trace.nettrace")
} else {
    output_path.to_owned()
};

Copilot AI linked an issue May 14, 2026 that may be closed by this pull request
getting the "os error 2" error when trying to write a trace #271
Closed
Copilot AI changed the title [WIP] Fix os error 2 when writing a trace Fix NetTrace output path resolution and fail-fast validation for record-trace May 14, 2026
Copilot AI requested a review from brianrob May 14, 2026 00:36
brianrob
brianrob reviewed May 14, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@brianrob brianrob left a comment

Choose a reason for hiding this comment

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

@copilot please address these comments.

Comment thread record-trace/engine/src/export.rs Outdated
return Err(anyhow!("{} is a directory.", output_path.display()));
}
warn!("NetTrace export path is a directory: path={}", output_path.display());
return Err(anyhow!("{} is a directory.", output_path.display()));
Copy link
Copy Markdown
Member

@brianrob brianrob May 14, 2026

Choose a reason for hiding this comment

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

This error should tell the user what to do - add a file name to the path (e.g. trace.nettrace).

Copy link
Copy Markdown
Contributor Author

Copilot AI May 14, 2026

Choose a reason for hiding this comment

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

Updated in commit 9cf3198. The directory-path validation error now includes clear guidance to provide a file path (for example, trace.nettrace).

Comment thread record-trace/engine/src/export.rs Outdated
if let Some(parent) = output_path.parent().filter(|parent| !parent.as_os_str().is_empty()) {
if !parent.exists() {
warn!("NetTrace export parent path does not exist: path={}", parent.display());
return Err(anyhow!("{} does not exist.", parent.display()));
Copy link
Copy Markdown
Member

@brianrob brianrob May 14, 2026

Choose a reason for hiding this comment

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

The error should be clearer - The output directory {} does not exist. Please create it and try again.

Copy link
Copy Markdown
Contributor Author

Copilot AI May 14, 2026

Choose a reason for hiding this comment

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

Updated in commit 9cf3198. The missing-directory error now says: "The output directory {} does not exist. Please create it and try again."

Comment thread record-trace/engine/src/export.rs Outdated
self.output_path.push("trace.nettrace");
else if !parent.is_dir() {
warn!("NetTrace export parent path is not a directory: path={}", parent.display());
return Err(anyhow!("{} is not a directory.", parent.display()));
Copy link
Copy Markdown
Member

@brianrob brianrob May 14, 2026

Choose a reason for hiding this comment

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

Same here - need to tell the user what to do to fix it.

Copy link
Copy Markdown
Contributor Author

Copilot AI May 14, 2026

Choose a reason for hiding this comment

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

Updated in commit 9cf3198. The parent-path-type error now includes actionable guidance: "The parent path {} exists but is not a directory. Please provide a path with a valid directory as the parent."

@brianrob brianrob marked this pull request as ready for review May 14, 2026 03:22
@brianrob brianrob requested a review from beaubelgrave May 14, 2026 03:23
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@copilot please address these comments.

Addressed in commit 9cf3198.

Copilot AI requested a review from brianrob May 14, 2026 03:25
beaubelgrave
beaubelgrave approved these changes May 14, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@beaubelgrave beaubelgrave left a comment

Choose a reason for hiding this comment

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

LGTM

@brianrob brianrob merged commit cfe3f78 into main May 14, 2026
18 checks passed
@brianrob brianrob deleted the copilot/fix-os-error-2-in-trace-writing branch May 14, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beaubelgrave beaubelgrave beaubelgrave approved these changes

@brianrob brianrob Awaiting requested review from brianrob

Assignees

@brianrob brianrob

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

getting the "os error 2" error when trying to write a trace

3 participants

@brianrob @beaubelgrave