From bf3d8a0a94e8a96472a582b0a7505c1f5faa706c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Wed, 7 Aug 2024 16:17:35 +0200
Subject: [PATCH] =?UTF-8?q?FIX:=20Don=E2=80=99t=20log=20an=20error=20when?=
 =?UTF-8?q?=20rendering=20a=20404?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, in the list controller, when encountering an unsafe redirect
error, a 404 is rendered. The problem is that it’s done in a way that it
also logs a fatal error (because a `Discourse::NotFound` exception was
raised inside a `rescue_from` block).

This patch addresses that issue by simply rendering a 404 without
raising any error.
---
 app/controllers/list_controller.rb    |  2 +-
 spec/requests/list_controller_spec.rb | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index cbdb6404604..89e602605eb 100644
--- a/app/controllers/list_controller.rb
+++ b/app/controllers/list_controller.rb
@@ -50,7 +50,7 @@ class ListController < ApplicationController
                 ].flatten
 
   rescue_from ActionController::Redirecting::UnsafeRedirectError do
-    raise Discourse::NotFound
+    rescue_discourse_actions(:not_found, 404)
   end
 
   # Create our filters
diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb
index 69f97f4e73a..4e86f5790a1 100644
--- a/spec/requests/list_controller_spec.rb
+++ b/spec/requests/list_controller_spec.rb
@@ -1227,17 +1227,27 @@ RSpec.describe ListController do
     end
 
     context "when redirect raises an unsafe redirect error" do
+      let(:fake_logger) { FakeLogger.new }
+
       before do
         ListController
           .any_instance
           .stubs(:redirect_to)
           .raises(ActionController::Redirecting::UnsafeRedirectError)
+        Rails.logger.broadcast_to(fake_logger)
       end
 
+      after { Rails.logger.stop_broadcasting_to(fake_logger) }
+
       it "renders a 404" do
         get "/c/hello/world/bye/#{subsubcategory.id}"
         expect(response).to have_http_status :not_found
       end
+
+      it "doesn’t log an error" do
+        get "/c/hello/world/bye/#{subsubcategory.id}"
+        expect(fake_logger.fatals).to be_empty
+      end
     end
 
     context "when provided slug is gibberish" do