From c1c8698b29edd473047cab9eb3f0198f3f407cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bu=CC=88nemann?= Date: Sun, 16 Feb 2020 15:59:49 +0100 Subject: [PATCH 1/3] Clear error_page state before each request This clears out the @error_page state before each request to avoid dirtying the error state of subsequent requests. For example a name error in the controller will cause an inspect call on the controller instance that will then also inspect the @error_page ivar referenced through the rack env in the controller. One possible side-effect of this are excessive database queries for unloaded relations. This also should reduce memory usage, since the app can garbage collect the no longer referenced state, delete temp files, etc. --- lib/better_errors/middleware.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/better_errors/middleware.rb b/lib/better_errors/middleware.rb index 262cf3b2..e25ebd2a 100644 --- a/lib/better_errors/middleware.rb +++ b/lib/better_errors/middleware.rb @@ -81,6 +81,7 @@ def better_errors_call(env) end def protected_app_call(env) + @error_page = nil @app.call env rescue Exception => ex @error_page = @handler.new ex, env From 42d63418e4e2c24a9b1affa0b407e4eb3355d6ea Mon Sep 17 00:00:00 2001 From: Robin Daugherty Date: Thu, 14 May 2020 14:57:05 -0400 Subject: [PATCH 2/3] Set error state aside while calling app --- lib/better_errors/middleware.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/better_errors/middleware.rb b/lib/better_errors/middleware.rb index e25ebd2a..51c00611 100644 --- a/lib/better_errors/middleware.rb +++ b/lib/better_errors/middleware.rb @@ -81,12 +81,22 @@ def better_errors_call(env) end def protected_app_call(env) + # Hold any existing error outside the instance that it's not going to pollute the `#inspect` of this instance + # while the app is being called. Once the app returns, restore the original error so that it is still available + # to Better Errors. If a new error is raised during the call, it be stored replacing the old one. + # + # This will have strange results in a multithreaded environment and another error is generated by a different + # thread while this app request is being called. + Thread.current[:better_errors_previous_error_page] = @error_page @error_page = nil @app.call env + @error_page = Thread.current[:better_errors_previous_error_page] rescue Exception => ex @error_page = @handler.new ex, env log_exception show_error_page(env, ex) + ensure + Thread.current[:better_errors_previous_error_page] = nil end def show_error_page(env, exception=nil) From 57b9c1566979208fc1cbdec3dc6b37f6c4c671f9 Mon Sep 17 00:00:00 2001 From: Robin Daugherty Date: Thu, 14 May 2020 15:00:54 -0400 Subject: [PATCH 3/3] Add fixme to solve threaded request issue --- lib/better_errors/middleware.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/better_errors/middleware.rb b/lib/better_errors/middleware.rb index 51c00611..2b3cff87 100644 --- a/lib/better_errors/middleware.rb +++ b/lib/better_errors/middleware.rb @@ -85,8 +85,10 @@ def protected_app_call(env) # while the app is being called. Once the app returns, restore the original error so that it is still available # to Better Errors. If a new error is raised during the call, it be stored replacing the old one. # - # This will have strange results in a multithreaded environment and another error is generated by a different - # thread while this app request is being called. + # FIXME: wrap this (and therefore all application requests) in a mutex. + # This will have strange results in a multithreaded environment. Either the second thread will clear + # the error state, or another thread request that rescues an exception will be overwritten by a slower + # request finishing after it. Thread.current[:better_errors_previous_error_page] = @error_page @error_page = nil @app.call env