Configurable timeout to render requests (#220)

Added/Changed
- Add `ssr_timeout` configuration so the Rails server will not wait more than 
   this many seconds for a SSR request to return once issued.
- Change default for `renderer_use_fallback_exec_js` to `false`.
- Change default log level to info.

Fixed
- Ability to call `server_render_js(raw_js)` fixed. Previously, always errored.
- Errors during rendering result in ReactOnRails::PrerenderError
- When retrying rendering, the retry message is more clear

Author: justin808

CHANGELOG.md

@@ -12,9 +12,27 @@ Gem and package versions are the same except for beta releases where the gem use
 ## [Unreleased]
 *Add changes in master not yet tagged.*
 
+### Added
+- Add `ssr_timeout` configuration so the Rails server will not wait more than this many seconds for a SSR request to return once issued.
+- Change default for `renderer_use_fallback_exec_js` to `false`.
+- Change default log level to info.
+  [PR 220](https://github.com/shakacode/react_on_rails_pro/pull/220) by
+  [justin808](https://github.com/justin808).
+
+- Add support for render functions to be async (returning promises). Also add `include_execjs_polyfills` option to configuration for React on Rails to optionally stop stubbing of setTimeout, setInterval, & clearTimeout polyfills while using NodeRenderer. [PR 210](https://github.com/shakacode/react_on_rails_pro/pull/210) by [judahmeek](https://github.com/judahmeek)
+
+### Fixed
+- Ability to call `server_render_js(raw_js)` fixed. Previously, always errored.
+- Errors during rendering result in ReactOnRails::PrerenderError
+- When retrying rendering, the retry message is more clear
+
 ## [2.3.0] - 2021-09-22
 
 ### Added
+- Configuration option for `ssr_timeout` so the Rails server will not wait more than this many seconds
+  for a SSR request to return once issued. Default timeout if not set is 5.
+  `config.ssr_timeout = 5`
+
 - Added optional method `extra_files_to_cache` to the definition of the module for the configuration of
   the remote_bundle_cache_adapter. This allows files outside of the regular build directory to be
   placed in the cached zip file and then extracted and restored when the cache is restored. The use
@@ -24,8 +42,6 @@ Gem and package versions are the same except for beta releases where the gem use
   [PR 221](https://github.com/shakacode/react_on_rails_pro/pull/221) by
   [justin808](https://github.com/justin808) and [ershadul1](https://github.com/ershadul1).
 
-  - Add support for render functions to be async (returning promises). Also add `include_execjs_polyfills` option to configuration for React on Rails to optionally stop stubbing of setTimeout, setInterval, & clearTimeout polyfills while using NodeRenderer. [PR 210](https://github.com/shakacode/react_on_rails_pro/pull/210) by [judahmeek](https://github.com/judahmeek)
-
 ## [2.2.0] - 2021-07-13
 - Change rake react_on_rails_pro:pre_stage_bundle_for_vm_renderer to use symlinks to save slug size. [PR 202](https://github.com/shakacode/react_on_rails_pro/pull/202) by [justin808](https://github.com/justin808).
 

Gemfile.development_dependencies

@@ -44,12 +44,12 @@ group :development do
 end
 
 group :development, :test do
-  gem "pry-byebug"
-  gem "pry", github: "pry/pry"
-  gem "pry-rails"
-  gem "pry-doc", github: "pry/pry-doc"
-  gem "pry-theme"
-  gem "pry-rescue"
+  gem 'pry', '>= 0.14.1' # Console with powerful introspection capabilities
+  # Need to use master of pry-byebug to use latest pry version
+  gem 'pry-byebug', github: 'Popmenu/pry-byebug' # Integrates pry with byebug
+  gem 'pry-doc' # Provide MRI Core documentation
+  gem 'pry-rails' # Causes rails console to open pry. `DISABLE_PRY_RAILS=1 rails c` can still open with IRB
+  gem 'pry-theme' # An easy way to customize Pry colors via theme files
 
   gem "rubocop", "1.18.3", require: false
   gem 'rubocop-performance', "1.11.4", require: false

docs/configuration.md

@@ -76,11 +76,16 @@ ReactOnRailsPro.configure do |config|
   # Default for `renderer_password` is ""
   # config.renderer_password = ENV["RENDERER_PASSWORD"]
 
+  # Set the `ssr_timeout` configuration so the Rails server will not wait more than this many seconds
+  # for a SSR request to return once issued. 
+  config.ssr_timeout = 5
+  
   # If false, then crash if no backup rendering when the remote renderer is not available
   # Can be useful to set to false in development or testing to make sure that the remote renderer
   # works and any non-availability of the remote renderer does not just do ExecJS.
-  # Default for `renderer_use_fallback_exec_js` is true.
-  config.renderer_use_fallback_exec_js = true
+  # Suggest setting this to false if the SSR JS code cannot run in ExecJS
+  # Default for `renderer_use_fallback_exec_js` is false.
+  config.renderer_use_fallback_exec_js = false
 
   # The maximum size of the http connection pool,
   # Set +pool_size+ to limit the maximum number of connections allowed.

lib/react_on_rails_pro/configuration.rb

@@ -20,6 +20,7 @@ def self.configuration
       dependency_globs: Configuration::DEFAULT_DEPENDENCY_GLOBS,
       excluded_dependency_globs: Configuration::DEFAULT_EXCLUDED_DEPENDENCY_GLOBS,
       remote_bundle_cache_adapter: Configuration::DEFAULT_REMOTE_BUNDLE_CACHE_ADAPTER,
+      ssr_timeout: Configuration::DEFAULT_SSR_TIMEOUT,
       ssr_pre_hook_js: nil,
       assets_to_copy: nil,
       renderer_request_retry_limit: Configuration::DEFAULT_RENDERER_REQUEST_RETRY_LIMIT,
@@ -36,6 +37,7 @@ class Configuration
     DEFAULT_RENDERER_HTTP_POOL_SIZE = 10
     DEFAULT_RENDERER_HTTP_POOL_TIMEOUT = 5
     DEFAULT_RENDERER_HTTP_POOL_WARN_TIMEOUT = 0.25
+    DEFAULT_SSR_TIMEOUT = 5
     DEFAULT_PRERENDER_CACHING = false
     DEFAULT_TRACING = false
     DEFAULT_DEPENDENCY_GLOBS = nil
@@ -51,15 +53,15 @@ class Configuration
                   :renderer_http_pool_size, :renderer_http_pool_timeout, :renderer_http_pool_warn_timeout,
                   :dependency_globs, :excluded_dependency_globs, :rendering_returns_promises,
                   :remote_bundle_cache_adapter, :ssr_pre_hook_js, :assets_to_copy,
-                  :renderer_request_retry_limit, :throw_js_errors
+                  :renderer_request_retry_limit, :throw_js_errors, :ssr_timeout
 
     def initialize(renderer_url: nil, renderer_password: nil, server_renderer: nil,
                    renderer_use_fallback_exec_js: nil, prerender_caching: nil,
                    renderer_http_pool_size: nil, renderer_http_pool_timeout: nil,
                    renderer_http_pool_warn_timeout: nil, tracing: nil, include_execjs_polyfills: nil,
                    dependency_globs: nil, excluded_dependency_globs: nil, rendering_returns_promises: nil,
                    remote_bundle_cache_adapter: nil, ssr_pre_hook_js: nil, assets_to_copy: nil,
-                   renderer_request_retry_limit: nil, throw_js_errors: nil)
+                   renderer_request_retry_limit: nil, throw_js_errors: nil, ssr_timeout: nil)
       self.renderer_url = renderer_url
       self.renderer_password = renderer_password
       self.server_renderer = server_renderer
@@ -78,6 +80,7 @@ def initialize(renderer_url: nil, renderer_password: nil, server_renderer: nil,
       self.assets_to_copy = assets_to_copy
       self.renderer_request_retry_limit = renderer_request_retry_limit
       self.throw_js_errors = throw_js_errors
+      self.ssr_timeout = ssr_timeout
     end
 
     def setup_config_values

lib/react_on_rails_pro/error.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
 module ReactOnRailsPro
-  class Error < StandardError
+  class Error < ::ReactOnRails::Error
   end
 end

lib/react_on_rails_pro/request.rb

@@ -48,11 +48,11 @@ def perform_request(path, form)
               raise ReactOnRailsPro::Error, "Time out error when getting the response on: #{path}.\n"\
                                             "Original error:\n#{e}\n#{e.backtrace}"
             end
-            available_retries -= 1
             Rails.logger.info do
-              "[ReactOnRailsPro] Timed out trying to connect to the Node Renderer.\
-               Retrying #{available_retries} more times..."
+              "[ReactOnRailsPro] Timed out trying to connect to the Node Renderer."\
+                " Retrying #{available_retries} more times..."
             end
+            available_retries -= 1
             next
           rescue StandardError => e
             raise ReactOnRailsPro::Error, "Can't connect to NodeRenderer renderer: #{path}.\n"\
@@ -142,6 +142,10 @@ def create_connection
           pool_size: ReactOnRailsPro.configuration.renderer_http_pool_size,
           pool_timeout: ReactOnRailsPro.configuration.renderer_http_pool_timeout,
           warn_timeout: ReactOnRailsPro.configuration.renderer_http_pool_warn_timeout,
+
+          # https://docs.ruby-lang.org/en/2.0.0/Net/HTTP.html#attribute-i-read_timeout
+          # https://github.com/bpardee/persistent_http/blob/master/lib/persistent_http/connection.rb#L168
+          read_timeout: ReactOnRailsPro.configuration.ssr_timeout,
           force_retry: true,
           url: ReactOnRailsPro.configuration.renderer_url
         )

lib/react_on_rails_pro/server_rendering_pool/node_rendering_pool.rb

@@ -52,6 +52,10 @@ def eval_js(js_code, render_options, send_bundle: false)
           ReactOnRailsPro::ServerRenderingPool::ProRendering
             .set_request_digest_on_render_options(js_code, render_options)
 
+          # In case this method is called with simple, raw JS, not depending on the bundle, next line
+          # is needed.
+          @bundle_update_utc_timestamp ||= bundle_utc_timestamp
+
           # TODO: Remove the request_digest. See https://github.com/shakacode/react_on_rails_pro/issues/119
           # From the request path
           # path = "/bundles/#{@bundle_update_utc_timestamp}/render"

packages/node-renderer/src/master.js

@@ -6,6 +6,7 @@ const cluster = require('cluster');
 const log = require('./shared/log');
 const { buildConfig, logSanitizedConfig } = require('./shared/configBuilder');
 const restartWorkers = require('./master/restartWorkers');
+const errorReporter = require('./shared/errorReporter');
 
 const MILLISECONDS_IN_MINUTE = 60000;
 
@@ -29,7 +30,10 @@ module.exports = function masterRun(runningConfig) {
     if (worker.isScheduledRestart) {
       log.info('Restarting worker #%d on schedule', worker.id);
     } else {
-      log.warn('Worker #%d died UNEXPECTEDLY :(, restarting', worker.id);
+      // TODO: Track last rendering request per worker.id
+      // TODO: Consider blocking a given rendering request if it kills a worker more than X times
+      const msg = `Worker ${worker.id} died UNEXPECTEDLY :(, restarting`;
+      errorReporter.notify(msg);
     }
     // Replace the dead worker:
     cluster.fork();

packages/node-renderer/src/worker.js

@@ -3,6 +3,9 @@
  * @module worker
  */
 
+// Set next value to true to test timeouts
+// const TESTING_TIMEOUTS = false
+
 const path = require('path');
 const cluster = require('cluster');
 const express = require('express');
@@ -27,6 +30,17 @@ const errorReporter = require('./shared/errorReporter');
 const tracing = require('./shared/tracing');
 const { lock, unlock } = require('./shared/locks');
 
+// Uncomment next 2 functions for testing timeouts
+// function sleep(ms) {
+//   return new Promise((resolve) => {
+//     setTimeout(resolve, ms);
+//   });
+// }
+//
+// function getRandomInt(max) {
+//   return Math.floor(Math.random() * Math.floor(max));
+// }
+
 function setHeaders(headers, res) {
   Object.keys(headers).forEach((key) => res.set(key, headers[key]));
 }
@@ -106,6 +120,16 @@ module.exports = function run(config) {
         return;
       }
 
+      // if(TESTING_TIMEOUTS && getRandomInt(2) === 1) {
+      //   console.log(
+      //     'ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ');
+      //   console.log(`Sleeping, to test timeouts`);
+      //   console.log(
+      //     'ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ');
+      //
+      //   await sleep(100000);
+      // }
+
       const { renderingRequest } = req.body;
       const { bundleTimestamp } = req.params;
       const { bundle: providedNewBundle, ...assetsToCopyObj } = req.files;

packages/node-renderer/src/worker/handleRenderRequest.js

@@ -208,7 +208,6 @@ module.exports = async function handleRenderRequest({
       error,
       'Caught top level error in handleRenderRequest',
     );
-    log.error(msg);
     errorReporter.notify(msg);
     return Promise.reject(error);
   }