[API Proposal]: Add FileNotFoundException.ThrowIfNotFound throw helper.

[AraHaan]

Background and motivation

Since .NET 6 when ThrowIfNull was added to ArgumentNullException, .NET has introduced helpful and more optimized public throw helpers which helps users with the following:

• those who made throw helpers manually might not be doing them correctly.
• helped improve code in dotnet/runtime itself as well which sped things up for everyone performance wise.
• In code where no throw helpers are used to help maximize performance of user's code.
• The fact that something like public throw helpers inside exception types are trivial in the runtime itself with very little problems.

API Proposal

namespace System.IO
{
    public class FileNotFoundException : IOException
    {
        // snip.
        // optional message otherwise use one provided by the runtime.
        public static void ThrowIfNotFound(string fileName, string message = "");
        // optional overload if accepted (could be called from the above version as well to make it a stub of this overload):
        public static void ThrowIfNotFound(bool found, string message = "");
    }
}

Implementation

namespace System.IO
{
    public class FileNotFoundException : IOException
    {
        // snip.
        public static void ThrowIfNotFound(string fileName, string message = "")
            => ThrowIfNotFound(File.Exists(fileName), message);
        public static void ThrowIfNotFound(bool found, string message = "")
        {
            if (!found)
            {
                ThrowIfNotFoundCore(message);
            }
        }
        private static void ThrowIfNotFoundCore(string message)
            => throw new FileNotFoundException(message);
    }
}

API Usage

public static string ReadFile(string fileName)
{
    // normally people would do this which is not as performant as this api being proposed:
    /*
    if (!File.Exists(fileName))
    {
        throw new FileNotFoundException("some message");
    }
    */
    FileNotFoundException.ThrowIfNotFound(fileName, "optional message for when file is not found.");
    return File.ReadAllText(fileName);
}

Alternative Designs

Same as above but require user themselves to pass in File.Exists(fileName) as first parameter instead of passing in the filename into the API. As such overloads for both has been added to the proposal just in case both are needed.

Risks

Minimal, it is expected that as .NET improves in major versions that new API's like this will get added. Also could help optimize people's code by suggesting people to use this over manually doing this thing and then not know why their code not as performant as they expected.

[jkotas]

FileNotFoundException.ThrowIfNotFound(fileName, "optional message for when file is not found.");
return File.ReadAllText(fileName);

This pattern has a race condition. A better way to provide a custom message for FileNotFoundException is to catch and rethrow the FileNotFoundException:

try
{
    return File.ReadAllText(fileName);
}
catch (FileNotFoundException e)
{
    throw new FileNotFoundException("custom message for when file is not found.");
}

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

Since .NET 6 when ThrowIfNull was added to ArgumentNullException, .NET has introduced helpful and more optimized public throw helpers which helps users with the following:

• those who made throw helpers manually might not be doing them correctly.
• helped improve code in dotnet/runtime itself as well which sped things up for everyone performance wise.
• In code where no throw helpers are used to help maximize performance of user's code.
• The fact that something like public throw helpers inside exception types are trivial in the runtime itself with very little problems.

API Proposal

namespace System.IO
{
    public class FileNotFoundException : IOException
    {
        // snip.
        // optional message otherwise use one provided by the runtime.
        public static void ThrowIfNotFound(string fileName, string message = "");
        // optional overload if accepted (could be called from the above version as well to make it a stub of this overload):
        public static void ThrowIfNotFound(bool found, string message = "");
    }
}

Implementation

namespace System.IO
{
    public class FileNotFoundException : IOException
    {
        // snip.
        public static void ThrowIfNotFound(string fileName, string message = "")
            => ThrowIfNotFound(File.Exists(fileName), message);
        public static void ThrowIfNotFound(bool found, string message = "")
        {
            if (!found)
            {
                ThrowIfNotFoundCore(message);
            }
        }
        private static void ThrowIfNotFoundCore(string message)
            => throw new FileNotFoundException(message);
    }
}

API Usage

public static string ReadFile(string fileName)
{
    // normally people would do this which is not as performant as this api being proposed:
    /*
    if (!File.Exists(fileName))
    {
        throw new FileNotFoundException("some message");
    }
    */
    FileNotFoundException.ThrowIfNotFound(fileName, "optional message for when file is not found.");
    return File.ReadAllText(fileName);
}

Alternative Designs

Same as above but require user themselves to pass in File.Exists(fileName) as first parameter instead of passing in the filename into the API. As such overloads for both has been added to the proposal just in case both are needed.

Risks

Minimal, it is expected that as .NET improves in major versions that new API's like this will get added. Also could help optimize people's code by suggesting people to use this over manually doing this thing and then not know why their code not as performant as they expected.

Author:	AraHaan
Assignees:	-
Labels:	api-suggestion, area-System.IO, untriaged, needs-area-label
Milestone:	-

[AraHaan]

FileNotFoundException.ThrowIfNotFound(fileName, "optional message for when file is not found.");
return File.ReadAllText(fileName);

This pattern has a race condition. A better way to provide a custom message for FileNotFoundException is to catch and rethrow the FileNotFoundException:

try
{
    return File.ReadAllText(fileName);
}
catch (FileNotFoundException e)
{
    throw new FileNotFoundException("custom message for when file is not found.");
}

Yeah the example code I made a little too simplified, there are a lot of people throwing FileNotFoundException themselves instead of having the runtime throwing it instead and catching it where they call it at to then add in their own message (probably because they want to try to optimize a bit by having the check done once before performing said calls). Although I do sort of agree that the message parameter in this proposal can be removed to enforce that they catch it and then rethrow it with their own custom message however, would it make it not optimize the methods as much due to the use of try/catch to add in their custom message when it comes to the JIT at runtime? This is something to consider as well.

[MichalPetryka]

#27217 would be the better alternative for reducing overhead here.

[jkotas]

they want to try to optimize a bit by having the check done once before performing said calls

Calling File.Exist and throwing exception before actually opening the file is a de-optimization. It results into extra OS sycalls that are the most expensive part. This pattern should be discouraged.

would it make it not optimize the methods

The OS syscalls to access the file are going to dominate the performance profile. This helper method would not be a material optimization. It would be just for convenience.

When introducing convenience helpers like this one, we typically look at how many places we would be able to use them in this repo. Do you see any places where this helper can be used in this repo? If you do not see many, it suggests that it is not worth introducing.

[Clockwork-Muse]

Calling File.Exist and throwing exception before actually opening the file is a de-optimization. It results into extra OS sycalls that are the most expensive part. This pattern should be discouraged.

It also doesn't really "solve" anything, because there's a variety of reasons that files can disappear at any point between the existence check and the actual open....

[colejohnson66]

I think the big problem here is that C#'s "exception-oriented failure" approach causes people to think: "put all error handling at the top, then all code below should be exception-free!". So, we end up with needless double checks like:

public string ReadFile(string path)
{
    if (!File.Exists(path))
        throw new MyCustomException("File does not exist.");
    return File.ReadAllText(path); // <-- "this will *never* throw because I verified it to exist!"
}

To wrap an exception, we must catch it and rethrow it. A proper writing of the above function would be:

public string ReadFile(string path)
{
    try
    {
        return File.ReadAllText(path);
    }
    catch (FileNotFoundException ex)
    {
        throw new MyCustomException("File does not exist.", ex);
    }
}

But now the function is twice as long. There's not really a nice solution. Perhaps an analyzer for unnecessary exception checks? In the first example, if I was just throwing a new FileNotFoundException before opening the file, the analyzer would tell the user to just remove the check:

public string ReadFile(string path)
{
    // analyzer would say to remove this check
    if (!File.Exists(path))
        throw new FileNotFoundException("File does not exist.");
    return File.ReadAllText(path);
}

[AraHaan]

I think the big problem here is that C#'s "exception-oriented failure" approach causes people to think: "put all error handling at the top, then all code below should be exception-free!". So, we end up with needless double checks like:

public string ReadFile(string path)
{
    if (!File.Exists(path))
        throw new MyCustomException("File does not exist.");
    return File.ReadAllText(path); // <-- "this will *never* throw because I verified it to exist!"
}

To wrap an exception, we must catch it and rethrow it. A proper writing of the above function would be:

public string ReadFile(string path)
{
    try
    {
        return File.ReadAllText(path);
    }
    catch (FileNotFoundException ex)
    {
        throw new MyCustomException("File does not exist.", ex);
    }
}

But now the function is twice as long. There's not really a nice solution. Perhaps an analyzer for unnecessary exception checks? In the first example, if I was just throwing a new FileNotFoundException before opening the file, the analyzer would tell the user to just remove the check:

public string ReadFile(string path)
{
    // analyzer would say to remove this check
    if (!File.Exists(path))
        throw new FileNotFoundException("File does not exist.");
    return File.ReadAllText(path);
}

Except there is use cases for that as well.

[jozkee]

I'm closing this due to the reasons already mentioned:

Calling File.Exist and throwing exception before actually opening the file is a de-optimization. It results into extra OS sycalls that are the most expensive part. This pattern should be discouraged.

It also doesn't really "solve" anything, because there's a variety of reasons that files can disappear at any point between the existence check and the actual open....