Skip to content

Commit

Permalink
Handle custom errors in Alchemy::Picture#url
Browse files Browse the repository at this point in the history
Currently we raise custom errors in the Alchemy::Picture#url
method without rescueing them later. This causes pages and the image library
to fail completely if one the images could not be rendered because the image file
is missing or the image format could not be used.

This is bad user experience. We should not display the broken image and not break the whole page.
  • Loading branch information
tvdeyen committed Aug 31, 2017
1 parent fb34bb1 commit 4dd2496
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Change Log

## 3.6.2 (unreleased)

* Handle custom errors in `Alchemy::Picture#url` [#1305](https://github.com/AlchemyCMS/alchemy_cms/pull/1305) by [tvdeyen](https://github.com/tvdeyen)

## 3.6.1 (2017-08-16)

* Do not ask `systempage?` everytime we load the page definition [#1239](https://github.com/AlchemyCMS/alchemy_cms/pull/1283) by [tvdeyen](https://github.com/tvdeyen)
Expand Down
10 changes: 9 additions & 1 deletion app/models/alchemy/picture/url.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module Alchemy
module Picture::Url
include Alchemy::Logger

TRANSFORMATION_OPTIONS = [
:crop,
:crop_from,
Expand Down Expand Up @@ -28,6 +30,9 @@ def url(options = {})
image = encoded_image(image, options)

image.url(options.except(*TRANSFORMATION_OPTIONS).merge(name: name))
rescue MissingImageFileError, WrongImageFormatError => e
log_warning e.message
nil
end

private
Expand All @@ -53,7 +58,10 @@ def processed_image(image, options = {})
#
def encoded_image(image, options = {})
target_format = options[:format] || default_render_format
raise WrongImageFormatError if !target_format.in?(Alchemy::Picture.allowed_filetypes)

unless target_format.in?(Alchemy::Picture.allowed_filetypes)
raise WrongImageFormatError.new(self, target_format)
end

options = {
flatten: target_format != 'gif' && image_file_format == 'gif'
Expand Down
7 changes: 6 additions & 1 deletion lib/alchemy/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,14 @@ class MissingImageFileError < StandardError; end

# Raised if calling +image_file+ on a Picture object returns nil.
class WrongImageFormatError < StandardError
def initialize(image, requested_format)
@image = image
@requested_format = requested_format
end

def message
allowed_filetypes = Alchemy::Picture.allowed_filetypes.map(&:upcase).to_sentence
"Requested image format is not one of allowed filetypes (#{allowed_filetypes})."
"Requested image format (#{@requested_format.inspect}) for #{@image.inspect} is not one of allowed filetypes (#{allowed_filetypes})."
end
end

Expand Down
18 changes: 14 additions & 4 deletions spec/models/alchemy/picture_url_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,13 @@ def decode_dragon_fly_job(url)
expect(picture).to receive(:image_file) { nil }
end

it 'raises error' do
expect { url }.to raise_error(Alchemy::MissingImageFileError)
it 'returns nil' do
expect(url).to be_nil
end

it "logs warning" do
expect(Alchemy::Logger).to receive(:warn)
url
end
end

Expand Down Expand Up @@ -171,8 +176,13 @@ def decode_dragon_fly_job(url)
{format: 'zip'}
end

it "raises error" do
expect { url }.to raise_error(Alchemy::WrongImageFormatError)
it "returns nil" do
expect(url).to be_nil
end

it "logs warning" do
expect(Alchemy::Logger).to receive(:warn)
url
end
end

Expand Down

0 comments on commit 4dd2496

Please sign in to comment.