Skip to content

Commit

Permalink
Accurately track any JSON Parser errors (#1086)
Browse files Browse the repository at this point in the history
* Accurately track any JSON Parser errors
* Provide the context of what JSON was received to Honeybadger and Raven
* Fix test for rails32, add extra value request_digest
  • Loading branch information
justin808 authored May 15, 2018
1 parent d9f5c38 commit ce5d298
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 18 deletions.
2 changes: 2 additions & 0 deletions lib/react_on_rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "rails"

require "react_on_rails/error"
require "react_on_rails/prerender_error"
require "react_on_rails/json_parse_error"
require "react_on_rails/react_on_rails_helper"
require "react_on_rails/controller"
require "react_on_rails/version"
Expand Down
26 changes: 26 additions & 0 deletions lib/react_on_rails/json_parse_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module ReactOnRails
class JsonParseError < ::ReactOnRails::Error
attr_reader :json

def initialize(parse_error, json)
@json = json
@original_error = parse_error
super(parse_error.message)
end

def to_honeybadger_context
to_error_context
end

def raven_context
to_error_context
end

def to_error_context
{
original_error: @original_error,
json: @json
}
end
end
end
23 changes: 13 additions & 10 deletions lib/react_on_rails/prerender_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ def raven_context
to_error_context
end

def to_error_context
result = {
component_name: component_name,
err: err,
props: props,
js_code: js_code,
console_messages: console_messages
}

result.merge!(err.to_error_context) if err.respond_to?(:to_error_context)
result
end

private

def calc_message(component_name, console_messages, err, js_code, props)
Expand Down Expand Up @@ -59,15 +72,5 @@ def calc_message(component_name, console_messages, err, js_code, props)
end
[backtrace, message]
end

def to_error_context
{
component_name: component_name,
err: err,
props: props,
js_code: js_code,
console_messages: console_messages
}
end
end
end
4 changes: 3 additions & 1 deletion lib/react_on_rails/react_component/render_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module ReactComponent
class RenderOptions
include Utils::Required

attr_accessor :request_digest

NO_PROPS = {}.freeze

def initialize(react_component_name: required("react_component_name"), options: required("options"))
Expand Down Expand Up @@ -49,7 +51,7 @@ def logging_on_server
end

def to_s
"{ react_component_name = #{react_component_name}, options = #{options}"
"{ react_component_name = #{react_component_name}, options = #{options}, request_digest = #{request_digest}"
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,13 @@ def exec_server_render_js(js_code, render_options, js_evaluator = nil)
"tmp/server-generated-#{@file_index % 10}.js")
@file_index += 1
end
json_string = js_evaluator.eval_js(js_code)
result = JSON.parse(json_string)
json_string = js_evaluator.eval_js(js_code, render_options)
result = nil
begin
result = JSON.parse(json_string)
rescue JSON::ParserError => e
raise ReactOnRails::JsonParseError.new(e, json_string)
end

if render_options.logging_on_server
console_script = result["consoleReplayScript"]
Expand Down Expand Up @@ -82,7 +87,7 @@ def trace_js_code_used(msg, js_code, file_name = "tmp/server-generated.js", forc
end
end

def eval_js(js_code)
def eval_js(js_code, _render_options)
@js_context_pool.with do |js_context|
result = js_context.eval(js_code)
js_context.eval("console.history = []")
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class ApplicationController < ActionController::Base
protect_from_forgery with: :exception

rescue_from ReactOnRails::PrerenderError do |err|
raise err if err.err.is_a?(ReactOnRails::JsonParseError)

Rails.logger.error("Caught ReactOnRails::PrerenderError in ApplicationController error handler.")
Rails.logger.error(err.message)
Rails.logger.error(err.backtrace.join("\n"))
Expand Down
10 changes: 10 additions & 0 deletions spec/dummy/spec/requests/server_render_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@
.to eq("Mr. Server Side Rendering")
end

it "generates a prerender error if invalid JSON returned" do
invalid_json = "{ some invalid JSON"
allow(ReactOnRails::ServerRenderingPool::RubyEmbeddedJavaScript)
.to receive(:eval_js).and_return(invalid_json)
expect { get server_side_hello_world_with_options_path }.to(raise_error do |error|
expect(error.raven_context[:json]).to eq(invalid_json)
expect(error.raven_context[:original_error]).to be_instance_of(JSON::ParserError)
end)
end

it "generates server rendered HTML if server renderering enabled for shared redux" do
get server_side_hello_world_shared_store_path
html_nodes = Nokogiri::HTML(response.body)
Expand Down
2 changes: 1 addition & 1 deletion spec/dummy_no_webpacker/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"react-dom": "^15.6.1",
"react-helmet": "^5.1.3",
"react-hot-loader": "^3.0.0-beta.6",
"react-on-rails": "10.1.4",
"react-on-rails": "11.0.5",
"react-proptypes": "^1.0.0",
"react-redux": "^5.0.5",
"react-router": "3.0.5",
Expand Down
6 changes: 3 additions & 3 deletions spec/dummy_no_webpacker/client/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5538,9 +5538,9 @@ react-hot-loader@^3.0.0-beta.6:
redbox-react "^1.3.6"
source-map "^0.6.1"

react-on-rails@10.1.4:
version "10.1.4"
resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-10.1.4.tgz#50e1b7596928746cff2ef90c00adb8e7cd41fe98"
react-on-rails@11.0.5:
version "11.0.5"
resolved "https://registry.yarnpkg.com/react-on-rails/-/react-on-rails-11.0.5.tgz#578a149ed860b8a0bbf71353466fc3a02dd31243"

react-proptypes@^1.0.0:
version "1.0.0"
Expand Down
1 change: 1 addition & 0 deletions spec/dummy_no_webpacker/config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
if using_rails32?
Dummy::Application.configure do
config.active_support.deprecation = :stderr
config.action_dispatch.show_exceptions = false
end
else
Rails.application.configure do
Expand Down

0 comments on commit ce5d298

Please sign in to comment.