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.

Sign up for GitHub

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

Jump to bottom

Change to_xml method options in the default error_formatter #290

Merged
merged 8 commits into from
Dec 13, 2012
Merged

Change to_xml method options in the default error_formatter #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3340e84
default xml error formatter root should be error instead of hash
dpsk Dec 13, 2012
86e244d
use old style hash
dpsk Dec 13, 2012
28e774f
add specs and changelog entry
dpsk Dec 13, 2012
67012da
use .to_s method in spec, to pass on all ruby platforms
dpsk Dec 13, 2012
b0f9d1b
add real xml specs and builder as dependency for this tests
dpsk Dec 13, 2012
ee76762
remove debug output
dpsk Dec 13, 2012
413727a
fix specs for multiple ruby platforms
dpsk Dec 13, 2012
7984e1f
another fix :)
dpsk Dec 13, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* [#277](https://github.com/intridea/grape/pull/277): Added a DSL to declare `formatter` in API settings - [@tim-vandecasteele](https://github.com/tim-vandecasteele).
* [#284](https://github.com/intridea/grape/pull/284): Added a DSL to declare `error_formatter` in API settings - [@dblock](https://github.com/dblock).
* [#285](https://github.com/intridea/grape/pull/285): Removed `error_format` from API settings, now matches request format - [@dblock](https://github.com/dblock).
* [#290](https://github.com/intridea/grape/pull/290): Add more readable root in xml `error_formatter` - [@dpsk](https://github.com/dpsk)
* Your contribution here.

0.2.2
Expand Down
1 change: 1 addition & 0 deletions grape.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Gem::Specification.new do |s|
s.add_development_dependency 'rack-test'
s.add_development_dependency 'rspec', '~> 2.9'
s.add_development_dependency 'bundler'
s.add_development_dependency 'builder'

s.files = `git ls-files`.split("\n")
s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n")
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/error_formatter/xml.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ module Xml
class << self

def call(message, backtrace, options = {})
result = message.is_a?(Hash) ? message : { :error => message }
result = message.is_a?(Hash) ? message : { :message => message }
if (options[:rescue_options] || {})[:backtrace] && backtrace && ! backtrace.empty?
result = result.merge({ :backtrace => backtrace })
end
result.respond_to?(:to_xml) ? result.to_xml : result.to_s
result.respond_to?(:to_xml) ? result.to_xml(:root => :error) : result.to_s
end

end
Expand Down
24 changes: 22 additions & 2 deletions spec/grape/middleware/exception_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'spec_helper'
require 'active_support/core_ext/hash'

describe Grape::Middleware::Error do

Expand Down Expand Up @@ -104,14 +105,33 @@ def app
'{"detail":"missing widget","error":"rain!"}'].should be_include(last_response.body)
end

it 'should be possible to return errors in xml format' do
@app ||= Rack::Builder.app do
use Grape::Middleware::Error, :rescue_all => true, :format => :xml
run ExceptionApp
end
get '/'
last_response.body.should == "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<error>\n <message>rain!</message>\n</error>\n"
end
Copy link
Member

Choose a reason for hiding this comment

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

I am confused. How is this XML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The object don't respond to the to_xml method, what you see is the output of to_s. For testing to_xml we need to add active_support and builder(https://github.com/lifo/docrails/blob/master/activesupport/lib/active_support/builder.rb) into dependencies, the question is should we?

Copy link
Member

Choose a reason for hiding this comment

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

Well, if we want to test XML, we definitely should!


it 'should be possible to return hash errors in xml format' do
@app ||= Rack::Builder.app do
use Grape::Middleware::Error, :rescue_all => true, :format => :xml
run ErrorHashApp
end
get '/'
["<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<error>\n <detail>missing widget</detail>\n <error>rain!</error>\n</error>\n",
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<error>\n <error>rain!</error>\n <detail>missing widget</detail>\n</error>\n"].should be_include(last_response.body)
end

it 'should be possible to specify a custom formatter' do
@app ||= Rack::Builder.app do
use Grape::Middleware::Error,
:rescue_all => true,
:format => :custom,
:error_formatters => {
:custom => lambda { |message, backtrace, options|
{ :custom_formatter => message }.inspect
:custom => lambda { |message, backtrace, options|
{ :custom_formatter => message }.inspect
}
}
run ExceptionApp
Expand Down