Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bitmap.Save 'A generic error occurred in GDI+ #31367

Closed
grahamehorner opened this issue Nov 1, 2019 · 10 comments
Closed

Bitmap.Save 'A generic error occurred in GDI+ #31367

grahamehorner opened this issue Nov 1, 2019 · 10 comments
Labels
area-System.Drawing help wanted [up-for-grabs] Good issue for external contributors untriaged New issue has not been triaged by the area owner
Milestone

Comments

@grahamehorner
Copy link

when calling bitmap.Save with a file path where the folder doesn't exist the save method throws a
System.Runtime.InteropServices.ExternalException: 'A generic error occurred in GDI+.'

IMHO the the folder should be created (to match folder file operations) or throw an expection of DirectoryNotFoundException

@danmoseley
Copy link
Member

Seems reasonable - do you want to offer a PR? We would want the same behavior on Unix and Windows, but if the directory was created in the .NET code, that would hopefully be true.

@safern
Copy link
Member

safern commented Nov 13, 2019

Seems reasonable to me as well.

@korsaire
Copy link

I can look into that.

@Backhage
Copy link

As a user I would expect an exception being thrown if I supplied a non-existing path. Creating folders is a side effect that might hide errors and lead to unexpected behavior.
Please consider changing the current exception to a DirectoryNotFoundException rather than trying to create the full path.

@danmoseley
Copy link
Member

What do other API do?

@grahamehorner
Copy link
Author

@danmosemsft File.Create creates the folder/directory structure

@korsaire
Copy link

@Backhage I agree with that. If the API change is approved, maybe later an optional bool param can be passed to indicate whether to create a non-existing path - but IMO it's out of scope of this issue.

@Backhage
Copy link

@grahamehorner At least for me, using .NET Core 3.0 on Windows, File.Create(@"C:\Temp\NonExistingPath\File.tmp"); throws a DirectoryNotFoundException.

@grahamehorner
Copy link
Author

grahamehorner commented Nov 24, 2019

@Backhage interesting that differs to my experiance with .NET 2.2 and yes your correct, .NET 3.0 throws the exception as I would have expected; Directory.Create also creates the directory tree?

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@Macromullet
Copy link
Contributor

The root of this issue is that Image.Save calls GdipSaveImageToFile, which is implemented natively. The only way to solve this issue is to introduce a new exception by doing the directory path check earlier. This check could potentially be moved, and only performed before the call to GdipSaveImageToFile, but it would then lead to a potential difference when the code paths diverge in Save(Stream stream, ImageCodecInfo encoder, EncoderParameters? encoderParams) when _rawData vs non-null.

@safern safern closed this as completed in 8981ad7 Apr 17, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
ViktorHofer pushed a commit to dotnet/winforms that referenced this issue Dec 5, 2022
…tnet/runtime#34998)

* Provide better exception when saving to non-existent directory

When saving an image to a non-existent directory, throw a DirectoryNotFoundException
vs a System.Runtime.InteropServices.ExternalException.

Fix dotnet/runtime#31367

* Change exception message to be more intuitive

Use: The directory {0} of the filename {1} does not exist.
instead of: The target directory {0} of the targetPath {1} does not exist.

Fix dotnet/runtime#31367

* Fix test on unix by using built path.

We were using a hardcoded path with backslashes. That didn't match the built path for the Assert.

* Make Test conditional as it requires GDI+

* Put directory check closer to where it's used to ensure that we
capture it from all Save variations.

* Remove trailing whitespace

* Update test to ensure it doesn't run on the .NET Framework

* Change CheckIfDirectoryExists to ThrowIfDirectoryDoesntExist
to match naming conventions for throw-style methods.

Fix dotnet/runtime#31367

* Fixes typo in test name

Fix dotnet/runtime#31367

Commit migrated from dotnet/runtime@8981ad7
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Drawing help wanted [up-for-grabs] Good issue for external contributors untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

8 participants