Don't throw exception for broken images, but return "empty" image info.

[bobdenotter]

Partial fix for bolt/bolt#6677

Just like when a file is 0 bytes, it should return an 'empty' image info, instead of throwing an exception.
The exception will just "break" the interface, and it won't tell the (end)user much.

Discussed this with Carson last night, and catching and filtering them out in the Bolt UI isn't optimal either, because it needs to be done in three places at least (the "files listing" page, the ajaxy listing in "pick from server" modals, and in the stack).

I'm for returning an "empty" image info, and present the user with a friendly warning in the user interface, that will also tell them which image is broken.

[bobdenotter]

If we agree on the general approach of this change, I'll fix the tests! :-)

[GwendolenLynch]

Well, I guess the same question has to be asked of the second exception in createSvgFromString() too … problem to battle, is the exceptions are really useful to know something went wrong.

… but not so useful for a stable UI. 😞

[CarsonF]

Generally I agree, but I think I'm going to switch sides for this case. We are just trying to display the info to the user, not really do anything with it (like make image modifications). An exception makes it hard for Twig since it doesn't handle it.

That said, we still need a way to determine if the image info is valid or not.
I think we should add an isValid(): bool method to Info. Both PHP and Twig can check for this and do whatever they need to do.

All the "empty" cases should be "invalid" too. Maybe rename (copy & deprecate) createEmpty method to createInvalid.

[CarsonF]

We should make sure thumbs handles the invalid object correctly instead of relying on an exception to be thrown.

[GwendolenLynch]

We should make sure thumbs handles the invalid object correctly instead of relying on an exception to be thrown.

bolt/thumbs#45

[CarsonF]

I added an createInvalid constructor method and deprecated createEmpty, since I don't think there's a case where we want to consider empty as valid.

[GwendolenLynch]

OK, the $valid on the constructor signature is really irking me, though setting it to true at that point is fine.

Thing is, the only time we care about changing that parameter is when we try to createFrom*(), and that need only happen in createInvalid() e.g.

public static function createInvalid()
{
    $invalid = new static(new Dimensions(0, 0), Type::unknown(), 0, 0, null, new Exif([]));
    $invalid->valid = false;

    return $invalid;
}

Note: Not marking approve/request as I'm happy to be convinced otherwise

[CarsonF]

Yeah that's fine. Feel free to change and merge.

[GwendolenLynch]

Changed, making sure I didn't miss something (valid) on test, then I have just enough time to get this over the line 👍