Skip to content

Conversation

@jjonescz
Copy link
Member

@RikkiGibson has noticed in the documentation of Environment.GetFolderPath that it might return an empty string. So this PR handles that case.

@jjonescz jjonescz requested a review from Copilot November 21, 2025 11:42
@jjonescz jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Nov 21, 2025
Copilot finished reviewing on behalf of jjonescz November 21, 2025 11:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds defensive error handling for a rare edge case where Environment.GetFolderPath or Path.GetTempPath() returns an empty string, preventing potential issues downstream when attempting to construct temporary file paths for file-based app artifacts.

Key Changes

  • Added a null/empty string check after obtaining the temporary directory path
  • Throws an InvalidOperationException with a descriptive message if the path is empty
  • Protects against potential ArgumentException or unexpected behavior when passing empty paths to Path.Join

@jjonescz jjonescz requested a review from a team November 21, 2025 13:57
@jjonescz jjonescz marked this pull request as ready for review November 21, 2025 13:57
</data>
</root>
<data name="EmptyTempPath" xml:space="preserve">
<value>Unable to determine a temporary directory path. Consider configuring the TEMP environment variable on Windows or local app data folder on Unix.</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Path.GetTempPath on Windows never returns an empty string. If GetTempPathW or GetTempPath2W returns a zero length, it throws instead. So this advice on "configuring the TEMP environment variable on Windows" will not be seen on Windows.

Copy link
Member

Choose a reason for hiding this comment

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

Might be worth having a unit test with different OSs so that we check this situation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Path.GetTempPath on Windows never returns an empty string.

So this advice on "configuring the TEMP environment variable on Windows" will not be seen on Windows.

Having a message that is potentially never applicable seems fine to me. I don't think it is guaranteed that Path.GetTempPath will never return an empty string in the future. The purpose of this change is to defend against bad values that would cause silent bugs down the road.

Might be worth having a unit test with different OSs so that we check this situation.

I'm not sure how can I create a unit test for this. Such test would probably need to reconfigure the OS so the empty temp path is returned.

@jjonescz jjonescz requested a review from MiYanni November 24, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-run-file Items related to the "dotnet run <file>" effort

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants