Skip to content

Conversation

@brianpopow
Copy link
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Change a few exceptions to InvalidImageContentException which were missed in #1179

@brianpopow brianpopow added the API label Apr 28, 2020
@brianpopow brianpopow added this to the 1.0.0-rc1 milestone Apr 28, 2020
@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #1190 into master will decrease coverage by 0.00%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1190      +/-   ##
==========================================
- Coverage   82.55%   82.54%   -0.01%     
==========================================
  Files         692      692              
  Lines       29976    29986      +10     
  Branches     3388     3388              
==========================================
+ Hits        24746    24752       +6     
- Misses       4533     4537       +4     
  Partials      697      697              
Flag Coverage Δ
#unittests 82.54% <73.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
src/ImageSharp/Formats/Gif/GifDecoderCore.cs 83.76% <ø> (ø)
src/ImageSharp/Formats/Gif/GifDecoder.cs 92.30% <50.00%> (-7.70%) ⬇️
src/ImageSharp/Formats/Jpeg/JpegDecoder.cs 92.85% <50.00%> (-7.15%) ⬇️
src/ImageSharp/Formats/Png/PngDecoder.cs 90.90% <50.00%> (-9.10%) ⬇️
src/ImageSharp/Formats/Tga/TgaDecoder.cs 90.90% <50.00%> (-9.10%) ⬇️
.../Common/Exceptions/InvalidImageContentException.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Bmp/BmpDecoder.cs 100.00% <100.00%> (ø)
src/ImageSharp/Formats/Gif/GifThrowHelper.cs 50.00% <100.00%> (+50.00%) ⬆️
src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs 87.50% <100.00%> (+1.78%) ⬆️
src/ImageSharp/Formats/Png/PngThrowHelper.cs 50.00% <100.00%> (+7.14%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5639bab...193dafe. Read the comment docs.

// TODO: use InvalidImageContentException here, if we decide to define it
// https://github.com/SixLabors/ImageSharp/issues/1110
throw new ImageFormatException($"Can not decode image. Failed to allocate buffers for possibly degenerate dimensions: {dims.Width}x{dims.Height}. This error can happen for very large RLE bitmaps, which are not supported.", ex);
throw new InvalidImageContentException($"Can not decode image. Failed to allocate buffers for possibly degenerate dimensions: {dims.Width}x{dims.Height}. This error can happen for very large RLE bitmaps, which are not supported.", ex);
Copy link
Member

Choose a reason for hiding this comment

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

Can you move these and others to the ThrowHelper classes for each format please. That's why I missed them.

GifThrowHelper.ThrowInvalidImageContentException($"Can not decode image. Failed to allocate buffers for possibly degenerate dimensions: {dims.Width}x{dims.Height}.", ex);

// Not reachable, as the previous statement will throw a exception.
return null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JimBobSquarePants is there a way to annotate a method, that it will always throw to avoid this return statement?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately not. There's something coming in NET5 I believe but it won't be backwards compatible.

@brianpopow brianpopow merged commit 39dd158 into master Apr 28, 2020
@brianpopow brianpopow deleted the invalidContentException branch April 28, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants