maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use custom, dynamic, error pages (#11744)

Browse source 
* Use ErrorsController to render error pages

* Hide GitHub bug report prompt from anything other than dev.to and forem.dev

* Update service worker to use 404 and 500

* Other fixes
This commit is contained in:
rhymes 2020-12-07 18:55:41 +01:00 • committed by GitHub
parent f6d73fea8d
commit 2ab0719501
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 111 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
app/controllers/errors_controller.rb Normal file
View file

@ -0,0 +1,27 @@
class ErrorsController < ApplicationController
GITHUB_BUG_REPORT_DOMAINS = ["dev.to", "forem.dev"].freeze
GITHUB_BUG_REPORT_URL = "https://github.com/forem/forem/issues/new?template=bug_report.md".freeze
# HTTP 404 - Not Found - https://httpstatuses.com/400
def not_found
render status: :not_found
end
# HTTP 422 - Unprocessable Entity - https://httpstatuses.com/422
def unprocessable_entity
render status: :unprocessable_entity
end
# HTTP 500 - Internal Server Error - https://httpstatuses.com/500
def internal_server_error
@github_bug_report_url = GITHUB_BUG_REPORT_URL
@display_github_bug_report_url = SiteConfig.app_domain.in?(GITHUB_BUG_REPORT_DOMAINS)
render status: :internal_server_error
end
# HTTP 503 - Service Unavailable - https://httpstatuses.com/503
def service_unavailable
render status: :service_unavailable
end
end

2
app/models/comment.rb
View file

@ -75,7 +75,7 @@ class Comment < ApplicationRecord
def path
"/#{user.username}/comment/#{id_code_generated}"
rescue StandardError
"/404.html"
"/404"
end
def parent_or_root_article

14
public/500.html → app/views/errors/internal_server_error.html.erb
View file

@ -62,7 +62,6 @@
</head>
<body>
<!-- This file lives in public/500.html -->
<div class="unhandled-error-page base-background-color">
<div class="dialog">
<div>
@ -70,7 +69,18 @@
<h1>Ooooops...</h1>
<h2>You have encountered an error.</h2>
</div>
<p style="margin: 10%">Our team has been notified. Please feel welcome to also submit a public <a href="https://github.com/thepracticaldev/dev.to/issues/new?template=bug_report.md">bug report</a>. <br><br/>We apologize for any inconvenience.</p>
<p style="margin: 10%">
Our team has been notified.
<% if @display_github_bug_report_url %>
Please feel welcome to also submit a public
<a href="<%= @github_bug_report_url %>" target="_blank" rel="noopener noreferer">bug report</a>.
<% end %>
<br><br />
We apologize for any inconvenience.
</p>
</div>
</div>
</body>

0
public/404.html → app/views/errors/not_found.html.erb
View file

0
public/503.html → app/views/errors/service_unavailable.html.erb
View file

1
public/422.html → app/views/errors/unprocessable_entity.html.erb
View file

@ -55,7 +55,6 @@
</head>
<body>
<!-- This file lives in public/422.html -->
<div class="dialog">
<div>
<h1>The change you wanted was rejected.</h1>

16
app/views/service_worker/index.js.erb
View file

@ -15,12 +15,12 @@
event.waitUntil(
caches.open(staticCacheName)
.then(cache => cache.addAll([
"/shell_top", // head, top bar, inline styles
"/shell_bottom", // footer
"/async_info/shell_version", // For comparing changes in the shell. Should be incremented with style changes.
"/404.html", // Not found page
"/500.html", // Error page
"/offline.html" //Offline page
"/shell_top", // head, top bar, inline styles
"/shell_bottom", // footer
"/async_info/shell_version", // For comparing changes in the shell. Should be incremented with style changes.
"/404", // Not found page
"/500", // Error page
"/offline.html" // Offline page
]))
);
});
@ -54,10 +54,10 @@
const endFetch = Promise.resolve(cachedShellBottom);
const middleFetch = fetch(url).then(response => {
if (!response.ok && response.status === 404) {
return caches.match('/404.html');
return caches.match('/404');
}
if (!response.ok && response.status != 404) {
return caches.match('/500.html');
return caches.match('/500');
}
return response;
}).catch(err => caches.match('/offline.html'));

4
config/application.rb
View file

@ -67,6 +67,10 @@ module PracticalDeveloper
# To improve security, Rails embeds the purpose and expiry metadata inside encrypted or signed cookies value.
config.action_dispatch.use_cookies_with_metadata = false
# Sets the exceptions application invoked by the ShowException middleware when an exception happens
# Defaults to ActionDispatch::PublicExceptions.new(Rails.public_path)
config.exceptions_app = routes # use ErrorsController to handle error pages
# After-initialize checker to add routes to reserved words
config.after_initialize do
Rails.application.reload_routes!

6
config/routes.rb
View file

@ -1,6 +1,12 @@
# rubocop:disable Metrics/BlockLength
Rails.application.routes.draw do
# errors routes
match "/404", to: "errors#not_found", via: :all, as: :errors_not_found
match "/422", to: "errors#unprocessable_entity", via: :all, as: :errors_unprocessable_entity
match "/500", to: "errors#internal_server_error", via: :all, as: :errors_internal_server_error
match "/503", to: "errors#service_unavailable", via: :all, as: :errors_service_unavailable
use_doorkeeper do
controllers tokens: "oauth/tokens"
end

53
spec/requests/errors_request_spec.rb Normal file
View file

@ -0,0 +1,53 @@
require "rails_helper"
RSpec.describe "Errors", type: :request do
describe "GET /404" do
it "returns not found error" do
get errors_not_found_path
expect(response).to have_http_status(:not_found)
end
end
describe "GET /422" do
it "returns unprocessable entity error" do
get errors_unprocessable_entity_path
expect(response).to have_http_status(:unprocessable_entity)
end
end
describe "GET /500" do
it "returns internal server error" do
get errors_internal_server_error_path
expect(response).to have_http_status(:internal_server_error)
end
it "does not include the prompt to report on Forem GitHub" do
allow(SiteConfig).to receive(:app_domain).and_return("example.com")
get errors_internal_server_error_path
expect(response).to have_http_status(:internal_server_error)
expect(response.body).not_to include("github.com/forem/forem/issues")
end
it "includes the prompt to report on Forem GitHub for DEV" do
allow(SiteConfig).to receive(:app_domain).and_return("dev.to")
get errors_internal_server_error_path
expect(response).to have_http_status(:internal_server_error)
expect(response.body).to include("github.com/forem/forem/issues")
end
end
describe "GET /503" do
it "returns service unavailable error" do
get errors_service_unavailable_path
expect(response).to have_http_status(:service_unavailable)
end
end
end