canonical / canonicalwebteam.blog

2 stars 11 forks source link

Handle image_template exception #148

Closed albertkol closed 4 years ago

albertkol commented 4 years ago

Change code to be able to handle situations where image has no src. If that's the case we will simply skip it.

Issue

Fixes https://github.com/canonical-web-and-design/ubuntu.com/issues/8087

QA

Browse to: https://snapcraft.io/blog/api/snap-posts/libreoffice It won't work.

Browse to: https://snapcraft-io-3018.demos.haus/blog/api/snap-posts/libreoffice It will work.

tbille commented 4 years ago

@albertkol could you add a bit more details to why it wasn't working (like an example image)

albertkol commented 4 years ago

@albertkol could you add a bit more details to why it wasn't working (like an example image)

@tbille I tried not to spend too much time on it. It is weird. The canonicalwebteam.image-template is the package that applies the cloudinary prefix to the images. image-template is called by the blog module that is looking to apply the cloudinary service to all the images it finds.

It looks like in some situations the images provided by the blog to the image-template have an invalid src. image-template has a condition where if the src is invalid it throws the exception url must contain a hostname which the blog modue was not catching therefore throwing a 500. So what I did is adding a try - catch to handle it. If the image-template conversion fails, I just want to leave the image raw.

I checked some failing links after the fix on snapcraft.io and ubuntu.com but I cannot see any raw images left unconverted, so I am not quite sure what images with invalid src it was trying to convert.

tbille commented 4 years ago

@albertkol This seems to be a bit of a "hacky" way of fixing this issue. We should dig a bit more in what is causing this error? Is it possible that the image.get("src") is fed with objects that are not images?

albertkol commented 4 years ago

@tbille will dig more then and follow up with new findings

albertkol commented 4 years ago

@tbille Apparently we have html like this: <figure class="wp-block-image"><img alt=""/></figure> where images have no src which causes the problem.

I will add a condition that will skip images that have no src. Is that a better solution?

tbille commented 4 years ago

This seems like a better solution. We still want to get exceptions in case something else fails in here.