Azure / azure-data-lake-store-net

Azure Data Lake Store .Net SDK
MIT License
18 stars 24 forks source link

Can't null check IEnumerable<DirectoryEntry> AdlsClient.EnumerateDirectory #34

Closed ghost closed 5 years ago

ghost commented 5 years ago

When Enumerating through directories if the filepath is wrong I am not able to check if the IEnumerable is empty or null. The code below crashes at line --if(files.Any())-- I believe it's because the internal class FileStatusOutput extends IEnumerator and is returning a null Enumerator but I am not able to check that because of access. You can easily replicate this by running the code below and passing in an invalid filepath. I could simply check if this filepath exist first but that seems unnecessary.

 IEnumerable<DirectoryEntry> files = dataLakeClient.EnumerateDirectory(filePath);

            if (files != null)
            {
                if (files.Any())
                {
                    return files.ToList();
                }
            }

            return null;

Here is the error log in regards to variable 'files'

System.NullReferenceException: 'Object reference not set to an instance of an object.'

rahuldutta90 commented 5 years ago

@dannykboyer Agree there is an issue here. The issue is we are throwing a nullreference exception whereas we should throw a adlsexception that says the file does not exist.

However the above check (files != null) to check if path exists or not is not correct in this context. The reason is we do not make a rest call when we return the enumerable. We make the rest call when the "MoveNext" of the enumerator is invoked. In your code that would be ".Any".

To ensure file exists or not at the time when we return the enumerable, we have to do a checkexists call internally (which is unecessary for a happy path, it is increasing the cost of enumeratedirectory by one extra rest api call).

When you call ".Any" we will throw an AdlsException if the path does not exist. So correct way to handle would be: IEnumerable files = dataLakeClient.EnumerateDirectory(filePath); try{

            if (files.Any())
            {
                return files.ToList();
            }
        }

catch(AdlsExcception ex){ if (ex.HttpStatus != HttpStatusCode.NotFound)//this is some unexpected error throw ex { } return null; } Will fix the issue to throw correct adlsexception in the next release

ghost commented 5 years ago

Ahh thanks for the explanation, I understand the process now. Thanks for the quick response, I have added some try and catches and will replace null exception with AdlsException once the update is released!

rahuldutta90 commented 5 years ago

Keeping it open so that we can track to fix the adlsexception issue.

rahuldutta90 commented 5 years ago

fixed this.