Display nested exceptions on the debug view #32410

yuki24 · 2018-04-02T02:18:23Z

This PR adds the ability to show nested exceptions, which I proposed a few weeks ago on the core mailing list.

It's probably easier to understand when you actually see it in action rather than explaining the entire thing, so here comes a screenshot:

I'm going to add the same functionality to the plain text response once this is merged.

rails-bot · 2018-04-02T02:18:38Z

r? @georgeclaghorn

(@rails-bot has picked a reviewer for you, use r? to override)

kaspth

I do like the idea of this. But how common are exception causes in development?

kaspth · 2018-04-02T14:21:40Z

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

+    end
+
+    def source_to_show_id
+      (traces[trace_to_show].first || {})[:id]


Hash(traces[trace_to_show].first)[:id]?

Possible, but I couldn't come up with strong advantages over (... || {}) (very basic syntax that beginners can understand while Hash(...) is a method call that could be overridden)

kaspth · 2018-04-02T14:24:43Z

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb

+    <div id="<%= cause.object_id %>" style="display: none;">
+      <% wrapper = ActionDispatch::ExceptionWrapper.new(@request.get_header("action_dispatch.backtrace_cleaner"), cause) %>
+      <%= render "rescues/source", source_extracts: wrapper.source_extracts, show_source_idx: wrapper.source_to_show_id, error_index: index %>
+      <%= render "rescues/trace", traces: wrapper.traces, trace_to_show: wrapper.trace_to_show, error_index: index %>


Why don't we just pass in a wrapper here and in this template? I'm not liking the wrapper instantiation in here.

Perhaps then we could do

@wrapper.wrapped_causes.each do |cause_wrapper| # … end

I thought about this too. The Enumerable would also remove the need of manually tracking the index. I'll implement this if we can get agreement with more folks.

kaspth · 2018-04-02T14:25:23Z

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb

+    </div>
+    <% index += 1 %>
+  <% end %>
+
  <%= render template: "rescues/_request_and_response" %>


Lets fix up this render to match the style of the others, while we're here.

I'd rather send a separate PR to not make the diff unnecessarily larger. Consistency doesn't seem critical in this particular case.

kaspth · 2018-04-02T14:25:29Z

actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb

-  <%= render template: "rescues/_source" %>
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %>
  <%= render template: "rescues/_request_and_response" %>


kaspth · 2018-04-02T14:25:36Z

actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb

-  <%= render template: "rescues/_source" %>
-  <%= render template: "rescues/_trace" %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %>
+  <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %>
  <%= render template: "rescues/_request_and_response" %>


kaspth · 2018-04-02T14:26:44Z

actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb

@@ -1,52 +1,54 @@
-<% names = @traces.keys %>
+<% names = traces.keys %>


With this we could spare the multiple instances for error_index: 0 in the render calls.

<% error_index = local_assigns.fetch(:error_index, 0) %>

yuki24 · 2018-04-02T14:49:59Z

how common are exception causes in development?

Any exception that originates from a rescue_from block has a cause. Also, almost any network-related exception has a cause when retry or some sort of re-connecting happens in a rescue block.

yuki24 · 2018-04-04T16:27:33Z

slightly updated the PR.

yuki24 · 2018-04-04T16:28:17Z

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

+        end
+
+        causes
+      end


I was wondering if there's a better way to convert a linked list to an Enumerable. Open to suggestions.

I was trying to fiddle around with Enumerator but couldn't really come up with something.

Maybe? (untested)

def causes_for(exception) return enum_for(__method__, exception) unless block_given? yield exception while exception = exception.cause end def wrapped_causes_for(exception, backtrace_cleaner) causes_for(exception).map { |cause| self.class.new(backtrace_cleaner, cause) } end

@matthewd tried it in my console, it works!

gsamokovarov · 2018-04-15T18:48:35Z

@yuki24 you may also wanna look into making a change in rails/web-console as it relies on the structure of the error page.

kaspth

I think this is pretty much good to go. @yuki24 can you check what this would mean for web-console?

kaspth · 2018-07-01T17:27:48Z

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

+        end
+
+        causes
+      end


I was trying to fiddle around with Enumerator but couldn't really come up with something.

yuki24 · 2018-07-06T03:29:29Z

@kaspth I haven't looked into it yet will do so if I could find time this weekend.

yuki24 · 2018-07-09T01:10:59Z

@gsamokovarov @kaspth here is a proof-of-concept branch: rails/web-console@master...yuki24:support-for-nested-exceptions

The current web-conole only points to the last exception. It'd have to be able to switch the exception to look at so it could properly switch the the binding. There's no test yet, but you should be able to see how it works and also how much of complexity needs to be added for nested exception support.

kaspth · 2018-07-15T10:33:56Z

@yuki24 that branch looks doable to me 😊 — for this PR have you tried @matthewd's wrapped causes suggestion yet? 😉

@gsamokovarov how does that seem to you? I'd like to move forward with this in the meantime 😊

gsamokovarov · 2018-07-15T18:53:22Z

@kaspth, please do. @yuki24 I have a few comments about the code structure, but feel free to open the PR and we'll work it out on the rails/web-console side.

yuki24 · 2018-07-16T00:49:13Z

@kaspth yes, I like @matthewd's idea. I'll update the PR to use it.

@gsamokovarov I'll send a PR shortly.

kaspth · 2018-07-20T19:20:35Z

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

+
+      def wrapped_causes_for(exception, backtrace_cleaner)
+        causes_for(exception).map { |cause| self.class.new(backtrace_cleaner, cause) }
+      end


Man, this looks clean now 😍

kaspth · 2018-07-20T19:21:38Z

Well done, thanks! 👏

rails-bot assigned georgeclaghorn Apr 2, 2018

kaspth reviewed Apr 2, 2018

View reviewed changes

yuki24 force-pushed the show-cause-on-debug-view-2 branch 2 times, most recently from 2f84af7 to b7af040 Compare April 3, 2018 15:44

yuki24 commented Apr 4, 2018

View reviewed changes

georgeclaghorn assigned kaspth and unassigned georgeclaghorn Apr 4, 2018

kaspth reviewed Jul 1, 2018

View reviewed changes

yuki24 force-pushed the show-cause-on-debug-view-2 branch 3 times, most recently from e9a84f7 to fb6f6dd Compare July 8, 2018 22:06

Show nested exceptions on the debug view

1f525b4

yuki24 force-pushed the show-cause-on-debug-view-2 branch from fb6f6dd to 1f525b4 Compare July 16, 2018 00:54

yuki24 mentioned this pull request Jul 16, 2018

Add support for nested exceptions rails/web-console#265

Merged

1 task

kaspth reviewed Jul 20, 2018

View reviewed changes

kaspth merged commit ba78b1f into rails:master Jul 20, 2018

yuki24 deleted the show-cause-on-debug-view-2 branch July 20, 2018 21:48

rafaelfranca mentioned this pull request Aug 7, 2018

In controller rescued error is shown if rescue clause raises #33414

Closed

yuki24 changed the title ~~Show nested exceptions on the debug view~~ Display nested exceptions on the debug view Oct 2, 2018

kamipo mentioned this pull request Oct 21, 2018

Use "show_source_idx" to find current source #25424

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display nested exceptions on the debug view #32410

Display nested exceptions on the debug view #32410

yuki24 commented Apr 2, 2018 •

edited

rails-bot commented Apr 2, 2018

kaspth left a comment

kaspth Apr 2, 2018

yuki24 Apr 2, 2018 •

edited

kaspth Apr 2, 2018

yuki24 Apr 2, 2018 •

edited

kaspth Apr 2, 2018

yuki24 Apr 2, 2018 •

edited

kaspth Apr 2, 2018

kaspth Apr 2, 2018

kaspth Apr 2, 2018

yuki24 commented Apr 2, 2018 •

edited

yuki24 commented Apr 4, 2018

yuki24 Apr 4, 2018 •

edited

kaspth Jul 1, 2018

matthewd Jul 6, 2018

kaspth Jul 6, 2018

gsamokovarov commented Apr 15, 2018 •

edited

kaspth left a comment

kaspth Jul 1, 2018

yuki24 commented Jul 6, 2018

yuki24 commented Jul 9, 2018

kaspth commented Jul 15, 2018

gsamokovarov commented Jul 15, 2018

yuki24 commented Jul 16, 2018

kaspth Jul 20, 2018

kaspth commented Jul 20, 2018

		@@ -1,52 +1,54 @@
		<% names = @traces.keys %>
		<% names = traces.keys %>

+                      end
+                      causes
+                    end

Display nested exceptions on the debug view #32410

Display nested exceptions on the debug view #32410

Conversation

yuki24 commented Apr 2, 2018 • edited

rails-bot commented Apr 2, 2018

kaspth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki24 Apr 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki24 Apr 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki24 Apr 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki24 commented Apr 2, 2018 • edited

yuki24 commented Apr 4, 2018

yuki24 Apr 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsamokovarov commented Apr 15, 2018 • edited

kaspth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki24 commented Jul 6, 2018

yuki24 commented Jul 9, 2018

kaspth commented Jul 15, 2018

gsamokovarov commented Jul 15, 2018

yuki24 commented Jul 16, 2018

Choose a reason for hiding this comment

kaspth commented Jul 20, 2018

yuki24 commented Apr 2, 2018 •

edited

yuki24 Apr 2, 2018 •

edited

yuki24 Apr 2, 2018 •

edited

yuki24 Apr 2, 2018 •

edited

yuki24 commented Apr 2, 2018 •

edited

yuki24 Apr 4, 2018 •

edited

gsamokovarov commented Apr 15, 2018 •

edited