Commit 5ebc4618 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'sh-log-422-responses' into 'master'

Log response body to production_json.log when a controller responds with a 422 status

See merge request gitlab-org/gitlab-ce!19473
parents 10fd34ba 5d3abdf9
...@@ -91,6 +91,10 @@ class ApplicationController < ActionController::Base ...@@ -91,6 +91,10 @@ class ApplicationController < ActionController::Base
payload[:user_id] = logged_user.try(:id) payload[:user_id] = logged_user.try(:id)
payload[:username] = logged_user.try(:username) payload[:username] = logged_user.try(:username)
end end
if response.status == 422 && response.body.present? && response.content_type == 'application/json'.freeze
payload[:response] = response.body
end
end end
# Controllers such as GitHttpController may use alternative methods # Controllers such as GitHttpController may use alternative methods
......
---
title: Log response body to production_json.log when a controller responds with a
422 status
merge_request:
author:
type: other
...@@ -27,6 +27,7 @@ unless Sidekiq.server? ...@@ -27,6 +27,7 @@ unless Sidekiq.server?
gitaly_calls = Gitlab::GitalyClient.get_request_count gitaly_calls = Gitlab::GitalyClient.get_request_count
payload[:gitaly_calls] = gitaly_calls if gitaly_calls > 0 payload[:gitaly_calls] = gitaly_calls if gitaly_calls > 0
payload[:response] = event.payload[:response] if event.payload[:response]
payload payload
end end
......
# coding: utf-8
require 'spec_helper' require 'spec_helper'
describe ApplicationController do describe ApplicationController do
...@@ -478,6 +479,63 @@ describe ApplicationController do ...@@ -478,6 +479,63 @@ describe ApplicationController do
end end
end end
describe '#append_info_to_payload' do
controller(described_class) do
attr_reader :last_payload
def index
render text: 'authenticated'
end
def append_info_to_payload(payload)
super
@last_payload = payload
end
end
it 'does not log errors with a 200 response' do
get :index
expect(controller.last_payload.has_key?(:response)).to be_falsey
end
context '422 errors' do
it 'logs a response with a string' do
response = spy(ActionDispatch::Response, status: 422, body: 'Hello world', content_type: 'application/json')
allow(controller).to receive(:response).and_return(response)
get :index
expect(controller.last_payload[:response]).to eq('Hello world')
end
it 'logs a response with an array' do
body = ['I want', 'my hat back']
response = spy(ActionDispatch::Response, status: 422, body: body, content_type: 'application/json')
allow(controller).to receive(:response).and_return(response)
get :index
expect(controller.last_payload[:response]).to eq(body)
end
it 'does not log a string with an empty body' do
response = spy(ActionDispatch::Response, status: 422, body: nil, content_type: 'application/json')
allow(controller).to receive(:response).and_return(response)
get :index
expect(controller.last_payload.has_key?(:response)).to be_falsey
end
it 'does not log an HTML body' do
response = spy(ActionDispatch::Response, status: 422, body: 'This is a test', content_type: 'application/html')
allow(controller).to receive(:response).and_return(response)
get :index
expect(controller.last_payload.has_key?(:response)).to be_falsey
end
end
end
describe '#access_denied' do describe '#access_denied' do
controller(described_class) do controller(described_class) do
def index def index
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment