diff --git a/Gemfile b/Gemfile index 25e1c15368..adbd35bc47 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,6 @@ gem 'jbuilder', '~> 2.7.0' gem 'figaro', '~> 1.1.1' gem 'hashie', '~> 3.5.7' gem 'aasm', '~> 5.0.0' -gem 'active_model-errors_details', '~> 1.3.1' gem 'bunny', '~> 2.11.0' gem 'cancancan', '~> 2.2.0' gem 'enumerize', '~> 2.2.2' diff --git a/Gemfile.lock b/Gemfile.lock index 883ea27d9d..01c694374b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -22,9 +22,6 @@ GEM erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.3) - active_model-errors_details (1.3.1) - activemodel (>= 3.2.13, < 5.0.0) - activesupport activejob (4.2.11) activesupport (= 4.2.11) globalid (>= 0.3.0) @@ -398,7 +395,6 @@ PLATFORMS DEPENDENCIES aasm (~> 5.0.0) - active_model-errors_details (~> 1.3.1) angularjs-rails (~> 1.3.15) annotate (~> 2.7) api-pagination (~> 4.8.2) diff --git a/app/api/v2/account/deposits.rb b/app/api/v2/account/deposits.rb index aa0b277341..63ddfbdb17 100644 --- a/app/api/v2/account/deposits.rb +++ b/app/api/v2/account/deposits.rb @@ -18,7 +18,7 @@ class Deposits < Grape::API optional :currency, type: String, values: { value: -> { Currency.enabled.codes(bothcase: true) }, message: 'account.currency.doesnt_exist' }, - desc: -> { "Currency value contains #{Currency.enabled.codes(bothcase: true).join(',')}" } + desc: 'Currency code' optional :state, type: String, values: { value: -> { Deposit::STATES.map(&:to_s) }, message: 'account.deposit.invalid_state' } @@ -52,9 +52,7 @@ class Deposits < Grape::API desc: "Deposit transaction id" end get "/deposits/:txid" do - deposit = current_user.deposits.find_by(txid: params[:txid]) - raise DepositByTxidNotFoundError, params[:txid] unless deposit - + deposit = current_user.deposits.find_by!(txid: params[:txid]) present deposit, with: API::V2::Entities::Deposit end diff --git a/app/api/v2/account/mount.rb b/app/api/v2/account/mount.rb index 93c9c97e28..3b60a04959 100644 --- a/app/api/v2/account/mount.rb +++ b/app/api/v2/account/mount.rb @@ -1,7 +1,6 @@ module API::V2 module Account class Mount < Grape::API - include NewExceptionsHandlers before { authenticate! } diff --git a/app/api/v2/account/withdraws.rb b/app/api/v2/account/withdraws.rb index 13085a246b..eae682285b 100644 --- a/app/api/v2/account/withdraws.rb +++ b/app/api/v2/account/withdraws.rb @@ -15,7 +15,7 @@ class Withdraws < Grape::API optional :currency, type: String, values: { value: -> { Currency.coins.enabled.codes(bothcase: true) }, message: 'account.currency.doesnt_exist'}, - desc: -> { "Any supported currencies: #{Currency.enabled.codes(bothcase: true).join(',')}." } + desc: 'Currency code.' optional :limit, type: { value: Integer, message: 'account.withdraw.non_integer_limit' }, values: { value: 1..100, message: 'account.withdraw.invalid_limit' }, @@ -73,7 +73,7 @@ class Withdraws < Grape::API rescue ::Account::AccountError => e report_exception_to_screen(e) - error!({ errors: ['account.withdraw.not_enough_funds'] }, 422) + error!({ errors: ['account.withdraw.insufficient_balance'] }, 422) rescue ActiveRecord::RecordInvalid => e report_exception_to_screen(e) error!({ errors: ['account.withdraw.invalid_amount'] }, 422) diff --git a/app/api/v2/errors.rb b/app/api/v2/errors.rb deleted file mode 100644 index 7900897941..0000000000 --- a/app/api/v2/errors.rb +++ /dev/null @@ -1,79 +0,0 @@ -# encoding: UTF-8 -# frozen_string_literal: true - -module API - module V2 - - module ExceptionHandlers - - def self.included(base) - base.instance_eval do - rescue_from Grape::Exceptions::ValidationErrors do |e| - error!({ error: { code: 1001, message: e.message } }, 422) - end - - rescue_from Peatio::Auth::Error do |e| - report_exception(e) - error!({ error: { code: e.code, message: 'Authorization failed' } }, 401) - end - - rescue_from ActiveRecord::RecordNotFound do |e| - error!('Couldn\'t find record.', 404) - end - end - end - end - - class Error < Grape::Exceptions::Base - attr :code, :text - - # code: api error code defined by Peatio, errors originated from - # subclasses of Error have code start from 2000. - # text: human readable error message - # status: http status code - def initialize(opts={}) - @code = opts[:code] || 2000 - @text = opts[:text] || '' - - @status = opts[:status] || 400 - @message = {error: {code: @code, message: @text}} - end - - def inspect - message = @text - message += " (#{@reason})" if @reason.present? - %[#<#{self.class.name}: #{message}>] - end - end - - # class CreateOrderError < Error - # def initialize(e) - # super code: 2002, text: 'Failed to create order.', status: 422 - # end - # end - - # class CancelOrderError < Error - # def initialize(e) - # super code: 2003, text: 'Failed to cancel order.', status: 422 - # end - # end - - # class OrderNotFoundError < Error - # def initialize(id) - # super code: 2004, text: "Order##{id} doesn't exist.", status: 404 - # end - # end - - # class CreateOrderAccountError < Error - # def initialize(e) - # super code: 2005, text: 'Not enough funds to create order.', status: 422 - # end - # end - - class DepositByTxidNotFoundError < Error - def initialize(txid) - super code: 2012, text: "Deposit##txid=#{txid} doesn't exist.", status: 404 - end - end - end -end diff --git a/app/api/v2/exception_handlers.rb b/app/api/v2/exception_handlers.rb new file mode 100644 index 0000000000..5a920a8eb1 --- /dev/null +++ b/app/api/v2/exception_handlers.rb @@ -0,0 +1,31 @@ +# encoding: UTF-8 +# frozen_string_literal: true + +module API::V2 + module ExceptionHandlers + def self.included(base) + base.instance_eval do + rescue_from Grape::Exceptions::ValidationErrors do |e| + errors_array = e.full_messages.map do |err| + err.split.last + end + error!({ errors: errors_array }, 422) + end + + rescue_from Peatio::Auth::Error do |e| + report_exception(e) + error!({ errors: ['jwt.decode_and_verify'] }, 401) + end + + rescue_from ActiveRecord::RecordNotFound do |_e| + error!({ errors: ['record.not_found'] }, 404) + end + + rescue_from :all do |e| + report_exception(e) + error!({ errors: ['server.internal_error'] }, 500) + end + end + end + end +end diff --git a/app/api/v2/helpers.rb b/app/api/v2/helpers.rb index d2b92c3cdf..e42603c074 100644 --- a/app/api/v2/helpers.rb +++ b/app/api/v2/helpers.rb @@ -71,7 +71,7 @@ def create_order(attrs) # TODO: Rewrite this rescue. rescue ::Account::AccountError => e report_exception_to_screen(e) - error!({ errors: ['market.account.not_enough_funds']}, 422) + error!({ errors: ['market.account.insufficient_balance']}, 422) rescue ::Order::InsufficientMarketLiquidity => e report_exception_to_screen(e) error!({ errors: ['market.order.insufficient_market_liquidity'] }, 422) diff --git a/app/api/v2/market/mount.rb b/app/api/v2/market/mount.rb index 25f45ea520..3663931ac1 100644 --- a/app/api/v2/market/mount.rb +++ b/app/api/v2/market/mount.rb @@ -3,7 +3,6 @@ module API::V2 module Market class Mount < Grape::API - include NewExceptionsHandlers before { authenticate! } before { trading_must_be_permitted! } diff --git a/app/api/v2/market/named_params.rb b/app/api/v2/market/named_params.rb new file mode 100644 index 0000000000..8130c0823f --- /dev/null +++ b/app/api/v2/market/named_params.rb @@ -0,0 +1,78 @@ +# encoding: UTF-8 +# frozen_string_literal: true + +module API + module V2 + module Market + module NamedParams + extend ::Grape::API::Helpers + + params :currency do + requires :currency, + type: String, + values: { value: -> { Currency.enabled.pluck(:id) }, message: 'account.currency.doesnt_exist' }, + desc: 'The currency code.' + end + + params :market do + requires :market, + type: String, + values: { value: -> { ::Market.enabled.ids }, message: 'market.market.doesnt_exist' }, + desc: -> { V2::Entities::Market.documentation[:id] } + end + + params :order do + requires :side, + type: String, + values: { value: %w(sell buy), message: 'market.order.invalid_side' }, + desc: -> { V2::Entities::Order.documentation[:side] } + requires :volume, + type: { value: BigDecimal, message: 'market.order.non_decimal_volume' }, + values: { value: -> (v){ v.try(:positive?) }, message: 'market.order.non_positive_volume' }, + desc: -> { V2::Entities::Order.documentation[:volume] } + optional :ord_type, + type: String, + values: { value: -> { Order::TYPES}, message: 'market.order.invalid_type' }, + default: 'limit', + desc: -> { V2::Entities::Order.documentation[:type] } + given ord_type: ->(val) { val == 'limit' } do + requires :price, + type: { value: BigDecimal, message: 'market.order.non_decimal_price' }, + values: { value: -> (p){ p.try(:positive?) }, message: 'market.order.non_positive_price' }, + desc: -> { V2::Entities::Order.documentation[:price] } + end + end + + params :order_id do + requires :id, + type: { value: Integer, message: 'market.order.non_integer_id' }, + allow_blank: { value: false, message: 'market.order.empty_id' }, + desc: -> { V2::Entities::Order.documentation[:id] } + end + + params :trade_filters do + optional :limit, + type: { value: Integer, message: 'market.trade.non_integer_limit' }, + values: { value: 1..1000, message: 'market.trade.invalid_limit' }, + default: 100, + desc: 'Limit the number of returned trades. Default to 100.' + optional :page, + type: { value: Integer, message: 'market.trade.non_integer_page' }, + allow_blank: { value: false, message: 'market.trade.empty_page' }, + default: 1, + desc: 'Specify the page of paginated results.' + optional :timestamp, + type: { value: Integer, message: 'market.trade.non_integer_timestamp' }, + allow_blank: { value: false, message: 'market.trade.empty_timestamp' }, + desc: "An integer represents the seconds elapsed since Unix epoch."\ + "If set, only trades executed before the time will be returned." + optional :order_by, + type: String, + values: { value: %w(asc desc), message: 'market.trade.invalid_order_by' }, + default: 'desc', + desc: "If set, returned trades will be sorted in specific order, default to 'desc'." + end + end + end + end +end diff --git a/app/api/v2/market/orders.rb b/app/api/v2/market/orders.rb index 0e4a37bc9b..e293a6cf58 100644 --- a/app/api/v2/market/orders.rb +++ b/app/api/v2/market/orders.rb @@ -5,7 +5,7 @@ module API module V2 module Market class Orders < Grape::API - helpers ::API::V2::NamedParams + helpers ::API::V2::Market::NamedParams desc 'Get your orders, results is paginated.', is_array: true, @@ -17,7 +17,7 @@ class Orders < Grape::API desc: -> { V2::Entities::Market.documentation[:id] } optional :state, type: String, - values: { value: -> { Order.state.values } , message: 'market.order.invalid_state' }, + values: { value: -> { Order.state.values }, message: 'market.order.invalid_state' }, desc: 'Filter order by state.' optional :limit, type: { value: Integer, message: 'market.order.non_integer_limit' }, diff --git a/app/api/v2/market/trades.rb b/app/api/v2/market/trades.rb index 80354857b8..3503300b53 100644 --- a/app/api/v2/market/trades.rb +++ b/app/api/v2/market/trades.rb @@ -5,7 +5,7 @@ module API module V2 module Market class Trades < Grape::API - helpers API::V2::NamedParams + helpers API::V2::Market::NamedParams desc 'Get your executed trades. Trades are sorted in reverse creation order.', is_array: true, diff --git a/app/api/v2/mount.rb b/app/api/v2/mount.rb index 6890bd729c..fc0243d78c 100644 --- a/app/api/v2/mount.rb +++ b/app/api/v2/mount.rb @@ -1,7 +1,7 @@ # encoding: UTF-8 # frozen_string_literal: true -require_dependency 'v2/errors' +require_dependency 'v2/exception_handlers' require_dependency 'v2/validations' module API diff --git a/app/api/v2/named_params.rb b/app/api/v2/named_params.rb deleted file mode 100644 index 4f3ac3a938..0000000000 --- a/app/api/v2/named_params.rb +++ /dev/null @@ -1,76 +0,0 @@ -# encoding: UTF-8 -# frozen_string_literal: true - -module API - module V2 - module NamedParams - extend ::Grape::API::Helpers - - params :currency do - requires :currency, - type: String, - values: { value: -> { Currency.enabled.pluck(:id) }, message: 'account.currency.doesnt_exist' }, - desc: 'The currency code.' - end - - params :market do - requires :market, - type: String, - values: { value: -> { ::Market.enabled.ids }, message: 'market.market.doesnt_exist' }, - desc: -> { V2::Entities::Market.documentation[:id] } - end - - params :order do - requires :side, - type: String, - values: { value: %w(sell buy), message: 'market.order.invalid_side' }, - desc: -> { V2::Entities::Order.documentation[:side] } - requires :volume, - type: { value: BigDecimal, message: 'market.order.non_decimal_volume' }, - values: { value: -> (v){ v.try(:positive?) }, message: 'market.order.non_positive_volume' }, - desc: -> { V2::Entities::Order.documentation[:volume] } - optional :ord_type, - type: String, - values: { value: -> { Order::TYPES}, message: 'market.order.invalid_type' }, - default: 'limit', - desc: -> { V2::Entities::Order.documentation[:type] } - given ord_type: ->(val) { val == 'limit' } do - requires :price, - type: { value: BigDecimal, message: 'market.order.non_decimal_price' }, - values: { value: -> (p){ p.try(:positive?) }, message: 'market.order.non_positive_price' }, - desc: -> { V2::Entities::Order.documentation[:price] } - end - end - - params :order_id do - requires :id, - type: { value: Integer, message: 'market.order.non_integer_id' }, - allow_blank: { value: false, message: 'market.order.empty_id' }, - desc: -> { V2::Entities::Order.documentation[:id] } - end - - params :trade_filters do - optional :limit, - type: { value: Integer, message: 'market.trade.non_integer_limit' }, - values: { value: 1..1000, message: 'market.trade.invalid_limit' }, - default: 100, - desc: 'Limit the number of returned trades. Default to 100.' - optional :page, - type: { value: Integer, message: 'market.trade.non_integer_page' }, - allow_blank: { value: false, message: 'market.trade.empty_page' }, - default: 1, - desc: 'Specify the page of paginated results.' - optional :timestamp, - type: { value: Integer, message: 'market.trade.non_integer_timestamp' }, - allow_blank: { value: false, message: 'market.trade.empty_timestamp' }, - desc: "An integer represents the seconds elapsed since Unix epoch."\ - "If set, only trades executed before the time will be returned." - optional :order_by, - type: String, - values: { value: %w(asc desc), message: 'market.trade.invalid_order_by' }, - default: 'desc', - desc: "If set, returned trades will be sorted in specific order, default to 'desc'." - end - end - end -end diff --git a/app/api/v2/new_exceptions_handlers.rb b/app/api/v2/new_exceptions_handlers.rb deleted file mode 100644 index c323076760..0000000000 --- a/app/api/v2/new_exceptions_handlers.rb +++ /dev/null @@ -1,25 +0,0 @@ -module API - module V2 - module NewExceptionsHandlers - def self.included(base) - base.instance_eval do - rescue_from Grape::Exceptions::ValidationErrors do |e| - errors_array = e.full_messages.map do |err| - err.split.last - end - error!({ errors: errors_array }, 422) - end - - rescue_from Peatio::Auth::Error do |e| - report_exception(e) - error!({ errors: ['jwt.decode_and_verify'] }, 401) - end - - rescue_from ActiveRecord::RecordNotFound do |_e| - error!({ errors: ['record.not_found'] }, 404) - end - end - end - end - end -end diff --git a/app/api/v2/public/currencies.rb b/app/api/v2/public/currencies.rb index 0b6ce577ac..4c380c85f7 100644 --- a/app/api/v2/public/currencies.rb +++ b/app/api/v2/public/currencies.rb @@ -10,7 +10,7 @@ class Currencies < Grape::API success Entities::Currency end params do - requires :id, + requires :id, type: String, values: { value: -> { Currency.enabled.codes(bothcase: true) }, message: 'public.currency.doesnt_exist'}, desc: -> { API::V2::Entities::Currency.documentation[:id][:desc] } diff --git a/app/api/v2/public/mount.rb b/app/api/v2/public/mount.rb index 02522408f5..f03fc0c38f 100644 --- a/app/api/v2/public/mount.rb +++ b/app/api/v2/public/mount.rb @@ -4,7 +4,6 @@ module API module V2 module Public class Mount < Grape::API - include NewExceptionsHandlers mount Public::Fees mount Public::Currencies diff --git a/spec/api/v2/account/deposits_spec.rb b/spec/api/v2/account/deposits_spec.rb index 63d30eeb87..a0f77f0487 100644 --- a/spec/api/v2/account/deposits_spec.rb +++ b/spec/api/v2/account/deposits_spec.rb @@ -83,11 +83,13 @@ it 'return 404 if txid not exist' do api_get '/api/v2/account/deposits/5', token: token expect(response.code).to eq '404' + expect(response).to include_api_error('record.not_found') end it 'returns 404 if txid not belongs_to you ' do api_get '/api/v2/account/deposits/10', token: token expect(response.code).to eq '404' + expect(response).to include_api_error('record.not_found') end it 'returns deposit txid if exist' do diff --git a/spec/api/v2/account/withdraws_spec.rb b/spec/api/v2/account/withdraws_spec.rb index b66a104c75..206b29d76b 100644 --- a/spec/api/v2/account/withdraws_spec.rb +++ b/spec/api/v2/account/withdraws_spec.rb @@ -175,7 +175,7 @@ data[:amount] = 100 api_post '/api/v2/account/withdraws', params: data, token: token expect(response).to have_http_status(422) - expect(response).to include_api_error('account.withdraw.not_enough_funds') + expect(response).to include_api_error('account.withdraw.insufficient_balance') end it 'validates type amount' do diff --git a/spec/api/v2/auth/middleware_spec.rb b/spec/api/v2/auth/middleware_spec.rb index 84588faabb..6c62cb615a 100644 --- a/spec/api/v2/auth/middleware_spec.rb +++ b/spec/api/v2/auth/middleware_spec.rb @@ -31,13 +31,13 @@ class Mount it 'should deny access when token is not given' do api_get '/api/v2/' expect(response.code).to eq '401' - expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' + expect(response).to include_api_error('jwt.decode_and_verify') end it 'should deny access when invalid token is given' do api_get '/api/v2/', token: '123.456.789' expect(response.code).to eq '401' - expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' + expect(response).to include_api_error('jwt.decode_and_verify') end it 'should allow access when valid token is given' do @@ -51,7 +51,7 @@ class Mount it 'should deny access' do api_get '/api/v2/' expect(response.code).to eq '401' - expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' + expect(response).to include_api_error('jwt.decode_and_verify') end end end diff --git a/spec/api/v2/helpers_spec.rb b/spec/api/v2/helpers_spec.rb index abe0348916..3e1632bd64 100644 --- a/spec/api/v2/helpers_spec.rb +++ b/spec/api/v2/helpers_spec.rb @@ -36,7 +36,7 @@ class Mount get '/api/v2/auth_test' expect(response.code).to eq '401' - expect(response.body).to eq '{"error":{"code":2001,"message":"Authorization failed"}}' + expect(response).to include_api_error('jwt.decode_and_verify') end end end diff --git a/spec/api/v2/market/orders_spec.rb b/spec/api/v2/market/orders_spec.rb index 303ee93281..4d449aaa0b 100644 --- a/spec/api/v2/market/orders_spec.rb +++ b/spec/api/v2/market/orders_spec.rb @@ -203,7 +203,7 @@ old_count = OrderAsk.count api_post '/api/v2/market/orders', token: token, params: { market: 'btcusd', side: 'sell', volume: '12.13', price: '2014' } expect(response.code).to eq '422' - expect(response).to include_api_error('market.account.not_enough_funds') + expect(response).to include_api_error('market.account.insufficient_balance') expect(OrderAsk.count).to eq old_count end @@ -239,7 +239,7 @@ api_post '/api/v2/market/orders', token: token, params: { market: 'btcusd', side: 'sell', volume: '1.0', ord_type: 'market' } expect(response.code).to eq '422' - expect(response).to include_api_error('market.account.not_enough_funds') + expect(response).to include_api_error('market.account.insufficient_balance') end it 'creates sell order' do diff --git a/spec/api/v2/mount_spec.rb b/spec/api/v2/mount_spec.rb index 6176311dee..4c1e6068c1 100644 --- a/spec/api/v2/mount_spec.rb +++ b/spec/api/v2/mount_spec.rb @@ -5,7 +5,9 @@ module API module V2 class Mount get('/null') { '' } - get('/broken') { raise Error, code: 2_014_310, text: 'MtGox bankrupt' } + get('/record-not-found') { raise ActiveRecord::RecordNotFound } + get('/auth-error') { raise Peatio::Auth::Error } + get('/standard-error') { raise StandardError } end end end @@ -17,10 +19,22 @@ class Mount end context 'handle exception on request processing' do - it 'should render json error message' do - get '/api/v2/broken' - expect(response.code).to eq '400' - expect(JSON.parse(response.body)).to eq('error' => { 'code' => 2_014_310, 'message' => 'MtGox bankrupt' }) + it 'returns array with record.not_found error' do + get '/api/v2/record-not-found' + expect(response.code).to eq '404' + expect(response).to include_api_error('record.not_found') + end + + it 'returns array with jwt.decode_and_verify error' do + get '/api/v2/auth-error' + expect(response.code).to eq '401' + expect(response).to include_api_error('jwt.decode_and_verify') + end + + it 'returns array with server.internal_error error' do + get '/api/v2/standard-error' + expect(response.code).to eq '500' + expect(response).to include_api_error('server.internal_error') end end