404 App not found + present option to init new app config and restart webserver

ericfranz commented 7 years ago

This is a rewriting of the original issue https://github.com/OSC/mod_ood_proxy/issues/7

Every time I load a new app for the first time my PUN is reloaded without warning, restarting running shell app instances

The solution is instead of restarting immediately, show a warning to the user, explaining the cost of restarting. This warning page can be presented to the user either in the pun config or in the /nginx/init handler. Clicking init would result in the new config being created if the app exists.

Ideally, we would we present 404 if the app does not exist. However, it would appear that Apache would be responsible for displaying this 404, but the PUN would be responsible for determining the problem, and we wouldn't want to use a redirect for the PUN to let Apache know this - rather, probably, custom headers and more complex proxying code than what we currently use. So we table that for now.

The simplest solution currently is to present the warning with the option to init the app in the PUN config.

We can modify https://github.com/OSC/nginx_stage/blob/dbdda7ad202350135a970bebfba379f638284f7c/templates/pun.conf.erb#L61-L65:

    <%- if app_init_url -%>
    location / {
-     return 302 <%= app_init_url %>;
+     default_type "text/html";
+     return 404 "<h1>App not found</h1>"
+                 <a href=\"<%= app_init_url %>\">Init App</a>";
    }
    <%- end -%>

Then if I go to a new app I see this:

screen 2017-09-24 at 2 58 11 pm

If I click the app link, and the app doesn't exist, I see this:

screen 2017-09-24 at 2 58 30 pm

But now this means that both responses are static HTML strings returned by NGINX or Apache lua module. So we can easily change these, especially since both NGINX and Lua allow you to add newlines to the strings.

The end result after determining the simplified HTML will look something like this:

screen 2017-09-24 at 3 01 30 pm

This HTML can be embedded in the nginx_stage repo and provided as a variable used when rendering the PUN.

So the result will be something more like:

    <%- if app_init_url -%>
    location / {
-     return 302 <%= app_init_url %>;
+     default_type "text/html";
+     return 404 <%= app_init_warning_html %>;
    }
    <%- end -%>

ericfranz commented 7 years ago

Files:

original.html.txt - Original mock message using CDN bootstrap:
inlined.html.txt - CSS inlined using https://templates.mailchimp.com/resources/inline-css/
inlined-simple.html.txt - Starting to remove some of the unecessary CSS rules from the inlined CSS:

More tools/tips on inlining CSS:

ericfranz commented 7 years ago

And the HTML template can be an erb as well, rendered using the <%= app_init_url %>, so this will make it easier to work with.

ericfranz commented 7 years ago

This erb should work:

<html>
<body style="font-family: 'Helvetica Neue', Helvetica,Arial,sans-serif;font-size: 16px;
  line-height: 1.4;color: #333;font-weight: 300;padding: 15px;">
<h2 style="font-weight: 500;font-size: 30px;">App has not yet been initialized or does not exist</h2>
<p style="color: #a94442;">Restarting your per-user web server will initialize
this app's config and make it available to you. This will reset any web socket
connections. If you have the Shell app open, please save your work prior to
clicking "Initialize App" below.</p>
<p>If the app has already been initialized, clicking "Initialize App" will
simply restart your web server, making the app available to you.</p>
<a href="<%= app_init_url %>" style="text-decoration: none;font-weight: 400;
  padding: 10px 16px;border-radius: 6px;color: #fff;background-color: #d9534f;">Initialize App</a>
</body>
</html>

Produces this:

screen 2017-09-25 at 10 11 10 am

Which may just be good enough. Render it to a string, using erb, then escape the result before using it in the pun config rendering:

rendered_html_string.gsub '"', '\"'

OSC / nginx_stage

404 App not found + present option to init new app config and restart webserver #20