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

Bug/Feature: Support fractions in the resize edits #254

Closed
MartijnKooij opened this issue Dec 29, 2020 · 8 comments
Closed

Bug/Feature: Support fractions in the resize edits #254

MartijnKooij opened this issue Dec 29, 2020 · 8 comments

Comments

@MartijnKooij
Copy link

Is your feature request related to a problem? Please describe.
With responsive pages it frequently happens for us that a desired image size will not become an integer value. When you request a resize edit with a decimal value for the width or height the service throws an exception.

{
    "message": "Internal error. Please contact the system administrator.",
    "code": "InternalError",
    "status": 500
}

Describe the feature you'd like
Apart from being quite hard to find, the error could be a bit more informative if this is indeed a hard requirement.
If possible downstream, the service should either do its best to actually support fractions when resizing, or to round the unsupported fractions for the client.

Additional context
Example of a resize edit with fractions.

{"bucket":"our.s3.bucket.org","key":"someimage.svg","edits":{"resize":{"width":37.5,"height":37.5,"fit":"fill"}}}
@beomseoklee
Copy link
Member

@MartijnKooij Thanks for your feedback.
Unfortunately, sharp does not support floating-point numbers for resizing: https://github.com/lovell/sharp/blob/master/lib/resize.js#L207

In the next release, we can maybe round up the width and height and enhance the error message if it's needed.

@MartijnKooij
Copy link
Author

It would be slightly more convenient to do the rounding server-side as opposed to client-side because in our case we have a couple of clients for the same server. But I'm not sure if that's the right solution for the serverless image handler in general, for all users, what do you think?

More meaningful errors would have saved me some digging around logs. So if that's easy to improve it would be appreciated.

@beomseoklee
Copy link
Member

@MartijnKooij we would definitely round up on the server side to deal in case any users provide the floating-point numbers on width and height.

@MartijnKooij
Copy link
Author

That would be awesome @beomseoklee !

About the error, I just noticed that in the lambda logs the error is already quite good ERROR Error: Expected positive integer for width but received 52.5 of type number. I was previously looking at the API Gateway logs only. So I'm now doubting whether it would be worth the change, I think it's more common to analyze these errors in the lambda logs right? API Gateway logging is also disabled by default (which is good).

@G-Lenz
Copy link
Contributor

G-Lenz commented Jan 29, 2021

We now round floating point numbers provided to the width and height on resize in v5.2.0.

@MartijnKooij
Copy link
Author

Awesome, thanks for the update!

@njtmead
Copy link

njtmead commented Mar 24, 2021

We now round floating point numbers provided to the width and height on resize in v5.2.0.

If possible could you please apply a similar change to overlayWith function left / top when entered as a percentage (ends with p).

It appears that after the fixed left / top is calculated from the percentage, the new value is not converted back to an integer and is throwing the same error message when the result is floating point.

@fisenkodv
Copy link
Contributor

@njtmead we have updated our solution, and I believe your issue has been fixed. If you still see the issue with the latest version (v6.0.0), please feel free to reopen the issue.

You can refer to the recent changes here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants