From ebd4140492da859b7dba9331ce9d0e2b188b479e Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 14 Feb 2019 17:58:16 +1100
Subject: [PATCH] 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.
---
 app/controllers/application_controller.rb    |  2 +-
 spec/requests/application_controller_spec.rb | 34 ++++++++++++++++++++
 spec/support/fake_logger.rb                  | 10 +++++-
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index e625b8e2c2b..e870e2bdc0a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -725,7 +725,7 @@ class ApplicationController < ActionController::Base
         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)
         @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)
       end
     end
diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb
index d2d63ab0102..966f99ac9ba 100644
--- a/spec/requests/application_controller_spec.rb
+++ b/spec/requests/application_controller_spec.rb
@@ -144,6 +144,40 @@ RSpec.describe ApplicationController do
         expect(response.body).to_not include('google.com/search')
       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
         $redis.del("page_not_found_topics")
 
diff --git a/spec/support/fake_logger.rb b/spec/support/fake_logger.rb
index 5a4c9f2acc1..ea89f06fa8b 100644
--- a/spec/support/fake_logger.rb
+++ b/spec/support/fake_logger.rb
@@ -1,10 +1,11 @@
 class FakeLogger
-  attr_reader :warnings, :errors, :infos
+  attr_reader :warnings, :errors, :infos, :fatals
 
   def initialize
     @warnings = []
     @errors = []
     @infos = []
+    @fatals = []
   end
 
   def info(message = nil)
@@ -18,4 +19,11 @@ class FakeLogger
   def error(message)
     @errors << message
   end
+
+  def fatal(message)
+    @fatals << message
+  end
+
+  def formatter
+  end
 end