FIX: logspam due to 404s on CSS files
We had a missing formats: string on our render partial that caused logs to spam when CSS files got 404s. Due to magic discourse_public_exceptions.rb was actually returning the correct 404 cause it switched format when rendering the error.
This commit is contained in:
parent
39522659a6
commit
ebd4140492
|
@ -725,7 +725,7 @@ class ApplicationController < ActionController::Base
|
||||||
category_topic_ids = Category.pluck(:topic_id).compact
|
category_topic_ids = Category.pluck(:topic_id).compact
|
||||||
@top_viewed = TopicQuery.new(nil, except_topic_ids: category_topic_ids).list_top_for("monthly").topics.first(10)
|
@top_viewed = TopicQuery.new(nil, except_topic_ids: category_topic_ids).list_top_for("monthly").topics.first(10)
|
||||||
@recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
|
@recent = Topic.includes(:category).where.not(id: category_topic_ids).recent(10)
|
||||||
@topics_partial = render_to_string partial: '/exceptions/not_found_topics'
|
@topics_partial = render_to_string partial: '/exceptions/not_found_topics', formats: [:html]
|
||||||
$redis.setex(key, 10.minutes, @topics_partial)
|
$redis.setex(key, 10.minutes, @topics_partial)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -144,6 +144,40 @@ RSpec.describe ApplicationController do
|
||||||
expect(response.body).to_not include('google.com/search')
|
expect(response.body).to_not include('google.com/search')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'no logspam' do
|
||||||
|
|
||||||
|
before do
|
||||||
|
@orig_logger = Rails.logger
|
||||||
|
Rails.logger = @fake_logger = FakeLogger.new
|
||||||
|
end
|
||||||
|
|
||||||
|
after do
|
||||||
|
Rails.logger = @orig_logger
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should handle 404 to a css file' do
|
||||||
|
|
||||||
|
$redis.del("page_not_found_topics")
|
||||||
|
|
||||||
|
topic1 = Fabricate(:topic)
|
||||||
|
get '/stylesheets/mobile_1_4cd559272273fe6d3c7db620c617d596a5fdf240.css', headers: { 'HTTP_ACCEPT' => 'text/css,*/*,q=0.1' }
|
||||||
|
expect(response.status).to eq(404)
|
||||||
|
expect(response.body).to include(topic1.title)
|
||||||
|
|
||||||
|
topic2 = Fabricate(:topic)
|
||||||
|
get '/stylesheets/mobile_1_4cd559272273fe6d3c7db620c617d596a5fdf240.css', headers: { 'HTTP_ACCEPT' => 'text/css,*/*,q=0.1' }
|
||||||
|
expect(response.status).to eq(404)
|
||||||
|
expect(response.body).to include(topic1.title)
|
||||||
|
expect(response.body).to_not include(topic2.title)
|
||||||
|
|
||||||
|
expect(Rails.logger.fatals.length).to eq(0)
|
||||||
|
expect(Rails.logger.errors.length).to eq(0)
|
||||||
|
expect(Rails.logger.warnings.length).to eq(0)
|
||||||
|
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
|
||||||
it 'should cache results' do
|
it 'should cache results' do
|
||||||
$redis.del("page_not_found_topics")
|
$redis.del("page_not_found_topics")
|
||||||
|
|
||||||
|
|
|
@ -1,10 +1,11 @@
|
||||||
class FakeLogger
|
class FakeLogger
|
||||||
attr_reader :warnings, :errors, :infos
|
attr_reader :warnings, :errors, :infos, :fatals
|
||||||
|
|
||||||
def initialize
|
def initialize
|
||||||
@warnings = []
|
@warnings = []
|
||||||
@errors = []
|
@errors = []
|
||||||
@infos = []
|
@infos = []
|
||||||
|
@fatals = []
|
||||||
end
|
end
|
||||||
|
|
||||||
def info(message = nil)
|
def info(message = nil)
|
||||||
|
@ -18,4 +19,11 @@ class FakeLogger
|
||||||
def error(message)
|
def error(message)
|
||||||
@errors << message
|
@errors << message
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def fatal(message)
|
||||||
|
@fatals << message
|
||||||
|
end
|
||||||
|
|
||||||
|
def formatter
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue