Merge pull request #1196 from kiwix/nicer_internal_server_error_page

Nicer Internal Server Error page

src/server/internalServer.cpp

@@ -709,12 +709,10 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
     return Response::build_redirect(contentUrl + query);
   } catch (std::exception& e) {
     fprintf(stderr, "===== Unhandled error : %s\n", e.what());
-    return HTTP500Response(request)
-         + ParameterizedMessage("non-translated-text", {{"MSG", e.what()}});
+    return HTTP500Response(request, m_root, request.get_full_url(), e.what());
   } catch (...) {
     fprintf(stderr, "===== Unhandled unknown error\n");
-    return HTTP500Response(request)
-         + nonParameterizedMessage("unknown-error");
+    return HTTP500Response(request, m_root, request.get_full_url());
   }
 }
 

src/server/response.cpp

@@ -491,15 +491,30 @@ HTTP400Response::HTTP400Response(const RequestContext& request)
   *this += ParameterizedMessage("invalid-request", {{"url", requestUrl}});
 }
 
-HTTP500Response::HTTP500Response(const RequestContext& request)
-  : HTTPErrorResponse(request,
-                      MHD_HTTP_INTERNAL_SERVER_ERROR,
-                      "500-page-title",
-                      "500-page-heading",
-                      std::string(),
-                      /*includeKiwixResponseData=*/true)
+HTTP500Response::HTTP500Response(const RequestContext& request,
+                                 const std::string& root,
+                                 const std::string& urlPath,
+                                 const std::string& errorText)
+  : ContentResponseBlueprint(&request,
+                             MHD_HTTP_INTERNAL_SERVER_ERROR,
+                             "text/html; charset=utf-8",
+                             RESOURCE::templates::sexy500_html,
+                             /*includeKiwixResponseData=*/true)
 {
-  *this += nonParameterizedMessage("500-page-text");
+  auto pageParams = Data::Object{
+    {"root", root },
+    {"url_path", urlPath},
+    {"PAGE_TITLE",   Data::fromMsgId("500-page-title")},
+    {"PAGE_HEADING", Data::fromMsgId("500-page-heading")},
+    {"PAGE_TEXT",    Data::fromMsgId("500-page-text")},
+    {"500_img_text", Data::fromMsgId("500-img-text")},
+  };
+
+  if ( !errorText.empty() ) {
+    pageParams["error"] = errorText;
+  }
+
+  *this->m_data = Data(pageParams);
 }
 
 std::unique_ptr<Response> Response::build_416(size_t resourceLength)

src/server/response.h

@@ -180,9 +180,12 @@ struct HTTP400Response : HTTPErrorResponse
   explicit HTTP400Response(const RequestContext& request);
 };
 
-struct HTTP500Response : HTTPErrorResponse
+struct HTTP500Response : ContentResponseBlueprint
 {
-  explicit HTTP500Response(const RequestContext& request);
+  HTTP500Response(const RequestContext& request,
+                  const std::string& root,
+                  const std::string& urlPath,
+                  const std::string& error = "");
 };
 
 class ItemResponse : public Response {

static/resources_list.txt

@@ -2,6 +2,7 @@ skin/caret.png
 skin/bittorrent.png
 skin/magnet.png
 skin/404.svg
+skin/500.svg
 skin/blocklink.svg
 skin/feed.svg
 skin/langSelector.svg
@@ -47,6 +48,7 @@ templates/viewer_settings.js
 templates/no_js_library_page.html
 templates/no_js_download.html
 templates/sexy404.html
+templates/sexy500.html
 opensearchdescription.xml
 ft_opensearchdescription.xml
 catalog_v2_searchdescription.xml

static/skin/i18n/en.json

@@ -30,8 +30,9 @@
 	, "404-advice.p4": "Look for keywords or titles related to the information you're seeking"
 	, "404-advice.p5": "This approach should help you locate the desired content, even if the original link isn't working properly."
 	, "500-page-title" : "Internal Server Error"
-	, "500-page-heading" : "Internal Server Error"
-	, "500-page-text": "An internal server error occured. We are sorry about that :/"
+	, "500-page-heading" : "Oops. Page isn't working."
+	, "500-page-text": "The requested path cannot be properly delivered:"
+	, "500-img-text": "Page isn't working"
 	, "external-link-detected" : "External Link Detected"
 	, "caution-warning" : "Caution!"
 	, "external-link-intro" : "You are about to leave Kiwix's ZIM reader to go online to"

static/skin/i18n/qqq.json

@@ -36,6 +36,7 @@
 	"500-page-title": "Title of the 500 error page",
 	"500-page-heading": "Heading of the 500 error page",
 	"500-page-text": "Text of the 500 error page",
+	"500-img-text": "Fallback text for the image on the 500 error page",
 	"external-link-detected": "Title & heading of the external link blocker page",
 	"caution-warning": "Warning of action that shouldn't be carried out carelessly",
 	"external-link-intro": "Message introducing the external link (to be followed by the actual link)",

static/skin/i18n/test.json

@@ -22,6 +22,10 @@
 	, "404-advice.p3": "[I18N TESTING] Check the spelling of the URL path"
 	, "404-advice.p4": "[I18N TESTING] Press the dice button"
 	, "404-advice.p5": "Good luck! [I18N TESTING]"
+	, "500-page-title" : "[I18N] Internal Server Error [TESTING]"
+	, "500-page-heading" : "Oops. [I18N] Page isn't [TESTING] working."
+	, "500-page-text": "The requested path [I18N TESTING] cannot be properly delivered:"
+	, "500-img-text": "Page [I18N] isn't [TESTING] working"
 	, "external-link-detected" : "External [I18] Link [TESTING] Detected"
 	, "caution-warning" : "[I18N] C5n! [TESTING]"
 	, "external-link-intro" : "[I18N TESTING] The following link may lead you to a place from which you won't ever be able to return"

static/templates/sexy500.html

@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
+    <title>{{PAGE_TITLE}}</title>
+    <link type="text/css" href="{{root}}/skin/error.css?KIWIXCACHEID" rel="Stylesheet" />
+    <script>
+      window.KIWIX_RESPONSE_TEMPLATE = "{{KIWIX_RESPONSE_TEMPLATE}}";
+      window.KIWIX_RESPONSE_DATA = {{{KIWIX_RESPONSE_DATA}}};
+    </script>
+  </head>
+  <body>
+    <header>
+        <img src="{{root}}/skin/500.svg?KIWIXCACHEID"
+             alt="{{500_img_text}}"
+             aria-label="{{500_img_text}}"
+             title="{{500_img_text}}">
+    </header>
+    <section class="intro">
+      <h1>{{PAGE_HEADING}}</h1>
+      <p>{{PAGE_TEXT}}</p>
+      <p><code>{{url_path}}</code></p>
+    </section>
+{{#error}}
+    <section class="advice">
+      <p>{{error}}</p>
+    </section>
+{{/error}}
+  </body>
+</html>

test/server.cpp

@@ -112,6 +112,8 @@ const ResourceCollection resources200Compressible{
 const ResourceCollection resources200Uncompressible{
   { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/404.svg" },
   { STATIC_CONTENT,  "/ROOT%23%3F/skin/404.svg?cacheid=b6d648af" },
+  { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/500.svg" },
+  { STATIC_CONTENT,  "/ROOT%23%3F/skin/500.svg?cacheid=32eb0f20" },
   { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/bittorrent.png" },
   { STATIC_CONTENT,  "/ROOT%23%3F/skin/bittorrent.png?cacheid=4f5c6882" },
   { DYNAMIC_CONTENT, "/ROOT%23%3F/skin/blank.html" },
@@ -1006,7 +1008,6 @@ std::string expectedSexy404ErrorHtml(const std::string& url)
 
 TEST_F(ServerTest, HttpSexy404HtmlError)
 {
-  using namespace TestingOfHtmlResponses;
   const std::vector<std::string> testUrls {
     // XXX: Nicer 404 error page no longer hints whether the error
     // XXX: is because of the missing book/ZIM-file or a missing article
@@ -1209,35 +1210,98 @@ TEST_F(ServerTest, HttpXmlError)
   }
 }
 
-TEST_F(ServerTest, 500)
+std::string expectedSexy500ErrorHtml(const std::string& url,
+                                     const std::string& error)
 {
-  const std::string expectedBody = R"(<!DOCTYPE html>
-<html xmlns="http://www.w3.org/1999/xhtml">
+  const auto urlWithoutQuery = replace(url, "\\?.*$", "");
+  const auto htmlSafeUrl = htmlEscape(urlWithoutQuery);
+  const auto jsSafeUrl = escapeJsString(urlWithoutQuery);
+
+  const std::string englishText[] = {
+    "Internal Server Error",
+    "Page isn&apos;t working",
+    "Oops. Page isn&apos;t working.",
+    "The requested path cannot be properly delivered:",
+  };
+
+  const std::string translatedText[] = {
+    "[I18N] Internal Server Error [TESTING]",
+    "Page [I18N] isn&apos;t [TESTING] working",
+    "Oops. [I18N] Page isn&apos;t [TESTING] working.",
+    "The requested path [I18N TESTING] cannot be properly delivered:",
+  };
+
+  const bool shouldTranslate = url.find("userlang=test") != std::string::npos;
+  const std::string* const t = shouldTranslate ? translatedText : englishText;
+
+  return R"RAWSTRINGLITERAL(<!DOCTYPE html>
+<html>
   <head>
-    <meta content="text/html;charset=UTF-8" http-equiv="content-type" />
-    <title>Internal Server Error</title>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
+    <title>)RAWSTRINGLITERAL" + t[0] + R"RAWSTRINGLITERAL(</title>
+    <link type="text/css" href="/ROOT%23%3F/skin/error.css?cacheid=b3fa90cf" rel="Stylesheet" />
     <script>
-      window.KIWIX_RESPONSE_TEMPLATE = )" + ERROR_HTML_TEMPLATE_JS_STRING + R"(;
-      window.KIWIX_RESPONSE_DATA = { "CSS_URL" : false, "PAGE_HEADING" : { "msgid" : "500-page-heading", "params" : { } }, "PAGE_TITLE" : { "msgid" : "500-page-title", "params" : { } }, "details" : [ { "p" : { "msgid" : "500-page-text", "params" : { } } }, { "p" : { "msgid" : "non-translated-text", "params" : { "MSG" : "Entry redirect_loop.html is a redirect entry." } } } ] };
+      window.KIWIX_RESPONSE_TEMPLATE = "&lt;!DOCTYPE html&gt;\n&lt;html&gt;\n  &lt;head&gt;\n    &lt;meta charset=&quot;utf-8&quot;&gt;\n    &lt;meta name=&quot;viewport&quot; content=&quot;width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no&quot; /&gt;\n    &lt;title&gt;{{PAGE_TITLE}}&lt;/title&gt;\n    &lt;link type=&quot;text/css&quot; href=&quot;{{root}}/skin/error.css?cacheid=b3fa90cf&quot; rel=&quot;Stylesheet&quot; /&gt;\n    &lt;script&gt;\n      window.KIWIX_RESPONSE_TEMPLATE = &quot;{{KIWIX_RESPONSE_TEMPLATE}}&quot;;\n      window.KIWIX_RESPONSE_DATA = {{{KIWIX_RESPONSE_DATA}}};\n    &lt;/script&gt;\n  &lt;/head&gt;\n  &lt;body&gt;\n    &lt;header&gt;\n        &lt;img src=&quot;{{root}}/skin/500.svg?cacheid=32eb0f20&quot;\n             alt=&quot;{{500_img_text}}&quot;\n             aria-label=&quot;{{500_img_text}}&quot;\n             title=&quot;{{500_img_text}}&quot;&gt;\n    &lt;/header&gt;\n    &lt;section class=&quot;intro&quot;&gt;\n      &lt;h1&gt;{{PAGE_HEADING}}&lt;/h1&gt;\n      &lt;p&gt;{{PAGE_TEXT}}&lt;/p&gt;\n      &lt;p&gt;&lt;code&gt;{{url_path}}&lt;/code&gt;&lt;/p&gt;\n    &lt;/section&gt;\n{{#error}}\n    &lt;section class=&quot;advice&quot;&gt;\n      &lt;p&gt;{{error}}&lt;/p&gt;\n    &lt;/section&gt;\n{{/error}}\n  &lt;/body&gt;\n&lt;/html&gt;\n";
+      window.KIWIX_RESPONSE_DATA = { "500_img_text" : { "msgid" : "500-img-text", "params" : { } }, "PAGE_HEADING" : { "msgid" : "500-page-heading", "params" : { } }, "PAGE_TEXT" : { "msgid" : "500-page-text", "params" : { } }, "PAGE_TITLE" : { "msgid" : "500-page-title", "params" : { } }, "error" : ")RAWSTRINGLITERAL"
+  +                     // inject the error
+  escapeJsString(error) // inject the error
+  +                     // inject the error
+  R"RAWSTRINGLITERAL(", "root" : "/ROOT%23%3F", "url_path" : ")RAWSTRINGLITERAL"
+  +         // inject the URL
+  jsSafeUrl // inject the URL
+  +         // inject the URL
+  R"RAWSTRINGLITERAL(" };
     </script>
   </head>
   <body>
-    <h1>Internal Server Error</h1>
-    <p>
-      An internal server error occured. We are sorry about that :/
-    </p>
-    <p>
-      Entry redirect_loop.html is a redirect entry.
-    </p>
+    <header>
+        <img src="/ROOT%23%3F/skin/500.svg?cacheid=32eb0f20"
+             alt=")RAWSTRINGLITERAL" + t[1] + R"RAWSTRINGLITERAL("
+             aria-label=")RAWSTRINGLITERAL" + t[1] + R"RAWSTRINGLITERAL("
+             title=")RAWSTRINGLITERAL" + t[1] + R"RAWSTRINGLITERAL(">
+    </header>
+    <section class="intro">
+      <h1>)RAWSTRINGLITERAL" + t[2] + R"RAWSTRINGLITERAL(</h1>
+      <p>)RAWSTRINGLITERAL" + t[3] + R"RAWSTRINGLITERAL(</p>
+      <p><code>)RAWSTRINGLITERAL"
+  +           // inject the URL
+  htmlSafeUrl // inject the URL
+  +           // inject the URL
+  R"RAWSTRINGLITERAL(</code></p>
+    </section>
+    <section class="advice">
+      <p>)RAWSTRINGLITERAL" + error + R"RAWSTRINGLITERAL(</p>
+    </section>
   </body>
 </html>
-)";
+)RAWSTRINGLITERAL";
+}
 
-  {
-  const auto r = zfs1_->GET("/ROOT%23%3F/content/poor/A/redirect_loop.html");
-  EXPECT_EQ(r->status, 500);
-  EXPECT_EQ(r->body, expectedBody);
-  EXPECT_EQ(r->get_header_value("Content-Type"), "text/html; charset=utf-8");
+TEST_F(ServerTest, 500)
+{
+  struct TestData {
+    const std::string url;
+    const std::string error;
+  };
+  const TestData testData[] = {
+    {
+      "/ROOT%23%3F/content/poor/A/redirect_loop.html",
+      "Entry redirect_loop.html is a redirect entry."
+    },
+
+    {
+      "/ROOT%23%3F/content/poor/A/redirect_loop.html?userlang=test",
+      "Entry redirect_loop.html is a redirect entry."
+    }
+  };
+
+  for ( const auto& td : testData ) {
+    const TestContext ctx{ {"url", td.url} };
+    const auto r = zfs1_->GET(td.url.c_str());
+    EXPECT_EQ(r->status, 500) << ctx;
+    EXPECT_EQ(r->get_header_value("Content-Type"), "text/html; charset=utf-8") << ctx;
+    EXPECT_EQ(r->body, expectedSexy500ErrorHtml(td.url, td.error)) << ctx;
   }
 }