From 0bee4c1222f7723f98377dc09ba8c156da1da185 Mon Sep 17 00:00:00 2001 From: st0012 Date: Mon, 11 Apr 2022 23:10:48 +0100 Subject: [PATCH 1/6] Use bp's class as condition instead of key type There are many types of Breakpoint uses an array as key. e.g. ISeqBreakpoint - `[:iseq, @iseq.path, @iseq.first_lineno]` So the current condition doesn't accurately selects line breakpoints. --- lib/debug/session.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/debug/session.rb b/lib/debug/session.rb index 627a3ebd5..82eb2663c 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -1346,7 +1346,7 @@ def add_line_breakpoint file, line, **kw def clear_line_breakpoints path path = resolve_path(path) @bps.delete_if do |k, bp| - if (Array === k) && DEBUGGER__.compare_path(k.first, path) + if bp.is_a?(LineBreakpoint) && DEBUGGER__.compare_path(k.first, path) bp.delete end end From c640900e3a1154a9ddbe9d0f63de656b7e7e849b Mon Sep 17 00:00:00 2001 From: st0012 Date: Tue, 12 Apr 2022 00:00:07 +0100 Subject: [PATCH 2/6] Protocol tests should show helpful info when failed to make requests Currently, if the request failed because the debugger exists early (broken pipe error), it doesn't provide the same information shown when a test fails. And that makes it hard to debug, so we should show helpful info in those cases as well. --- test/support/protocol_utils.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/support/protocol_utils.rb b/test/support/protocol_utils.rb index cab282f77..968338293 100644 --- a/test/support/protocol_utils.rb +++ b/test/support/protocol_utils.rb @@ -494,6 +494,8 @@ def send_request command, **kw send method: command, params: kw end + rescue StandardError => e + flunk create_protocol_message "Failed to send request because of #{e.inspect}" end def send_dap_request command, **kw From 09e8075d3269e05f9631e88539f45de5fbd8d643 Mon Sep 17 00:00:00 2001 From: st0012 Date: Tue, 12 Apr 2022 19:03:25 +0100 Subject: [PATCH 3/6] Fix raw tests' setExceptionBreakpoints response It should return either `nil` (if bp is duplicated) or the `bp.inspect`. Currently it returns `true` for duplicated because of `bp.disable`. --- lib/debug/session.rb | 5 ++++- test/protocol/catch_raw_dap_test.rb | 6 ++++-- test/protocol/detach_raw_dap_test.rb | 6 ++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/debug/session.rb b/lib/debug/session.rb index 82eb2663c..be0bc73eb 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -1235,9 +1235,12 @@ def add_bp bp @repl_prev_line = nil if @bps.has_key? bp.key - unless bp.duplicable? + if bp.duplicable? + bp + else @ui.puts "duplicated breakpoint: #{bp}" bp.disable + nil end else @bps[bp.key] = bp diff --git a/test/protocol/catch_raw_dap_test.rb b/test/protocol/catch_raw_dap_test.rb index 29953e35a..efe1cb8c1 100644 --- a/test/protocol/catch_raw_dap_test.rb +++ b/test/protocol/catch_raw_dap_test.rb @@ -569,9 +569,11 @@ def test_catching_any_exception_works_correctly verified: true, message: /# Date: Tue, 12 Apr 2022 18:30:50 +0100 Subject: [PATCH 4/6] Improve DAP's setExceptionBreakpoints request 1. Allow unset exception breakpoints by sending empty filters. 2. Support adding condition to exception breakpoints. --- lib/debug/server_dap.rb | 29 +++++++++++--------- lib/debug/session.rb | 12 +++++++-- test/protocol/catch_raw_dap_test.rb | 6 ++--- test/protocol/catch_test.rb | 40 +++++++++++++++++++++------- test/protocol/detach_raw_dap_test.rb | 6 ++--- test/support/protocol_utils.rb | 18 +++++++------ 6 files changed, 70 insertions(+), 41 deletions(-) diff --git a/lib/debug/server_dap.rb b/lib/debug/server_dap.rb index 0f3893d7b..fcb7d1085 100644 --- a/lib/debug/server_dap.rb +++ b/lib/debug/server_dap.rb @@ -107,14 +107,14 @@ def dap_setup bytes { filter: 'any', label: 'rescue any exception', - #supportsCondition: true, + supportsCondition: true, #conditionDescription: '', }, { filter: 'RuntimeError', label: 'rescue RuntimeError', default: true, - #supportsCondition: true, + supportsCondition: true, #conditionDescription: '', }, ], @@ -252,27 +252,30 @@ def process when 'setFunctionBreakpoints' send_response req when 'setExceptionBreakpoints' - process_filter = ->(filter_id) { - case filter_id - when 'any' - bp = SESSION.add_catch_breakpoint 'Exception' - when 'RuntimeError' - bp = SESSION.add_catch_breakpoint 'RuntimeError' - else - bp = nil - end + process_filter = ->(filter_id, cond = nil) { + bp = + case filter_id + when 'any' + SESSION.add_catch_breakpoint 'Exception', cond: cond + when 'RuntimeError' + SESSION.add_catch_breakpoint 'RuntimeError', cond: cond + else + nil + end { - verified: bp ? true : false, + verified: !bp.nil?, message: bp.inspect, } } + SESSION.clear_catch_breakpoints 'Exception', 'RuntimeError' + filters = args.fetch('filters').map {|filter_id| process_filter.call(filter_id) } filters += args.fetch('filterOptions', {}).map{|bp_info| - process_filter.call(bp_info.dig('filterId')) + process_filter.call(bp_info['filterId'], bp_info['condition']) } send_response req, breakpoints: filters diff --git a/lib/debug/session.rb b/lib/debug/session.rb index be0bc73eb..1c547a288 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -1327,8 +1327,8 @@ def repl_add_watch_breakpoint arg @tc << [:breakpoint, :watch, expr[:sig], cond, cmd, path] end - def add_catch_breakpoint pat - bp = CatchBreakpoint.new(pat) + def add_catch_breakpoint pat, cond: nil + bp = CatchBreakpoint.new(pat, cond: cond) add_bp bp end @@ -1355,6 +1355,14 @@ def clear_line_breakpoints path end end + def clear_catch_breakpoints *exception_names + @bps.delete_if do |k, bp| + if bp.is_a?(CatchBreakpoint) && exception_names.include?(k[1]) + bp.delete + end + end + end + def add_iseq_breakpoint iseq, **kw bp = ISeqBreakpoint.new(iseq, [:line], **kw) add_bp bp diff --git a/test/protocol/catch_raw_dap_test.rb b/test/protocol/catch_raw_dap_test.rb index efe1cb8c1..27e5f76c5 100644 --- a/test/protocol/catch_raw_dap_test.rb +++ b/test/protocol/catch_raw_dap_test.rb @@ -569,11 +569,9 @@ def test_catching_any_exception_works_correctly verified: true, message: /# Date: Tue, 12 Apr 2022 18:36:39 +0100 Subject: [PATCH 5/6] Refactor Session.clear_*_breakpoints methods --- lib/debug/session.rb | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/debug/session.rb b/lib/debug/session.rb index 1c547a288..a588a64eb 100644 --- a/lib/debug/session.rb +++ b/lib/debug/session.rb @@ -1346,20 +1346,25 @@ def add_line_breakpoint file, line, **kw @ui.puts e.message end - def clear_line_breakpoints path - path = resolve_path(path) + def clear_breakpoints(&condition) @bps.delete_if do |k, bp| - if bp.is_a?(LineBreakpoint) && DEBUGGER__.compare_path(k.first, path) + if condition.call(k, bp) bp.delete + true end end end + def clear_line_breakpoints path + path = resolve_path(path) + clear_breakpoints do |k, bp| + bp.is_a?(LineBreakpoint) && DEBUGGER__.compare_path(k.first, path) + end + end + def clear_catch_breakpoints *exception_names - @bps.delete_if do |k, bp| - if bp.is_a?(CatchBreakpoint) && exception_names.include?(k[1]) - bp.delete - end + clear_breakpoints do |k, bp| + bp.is_a?(CatchBreakpoint) && exception_names.include?(k[1]) end end From 8ae2b1007ac14412f2f4d851e268ae117a034054 Mon Sep 17 00:00:00 2001 From: st0012 Date: Tue, 12 Apr 2022 18:56:51 +0100 Subject: [PATCH 6/6] Improve req_set_exception_breakpoints helper --- CONTRIBUTING.md | 25 +++++++++++++++++++++++-- test/protocol/catch_test.rb | 15 +++++++-------- test/support/protocol_utils.rb | 12 +++++------- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b9fa7f1a3..fa535f711 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -316,9 +316,30 @@ Sends request to rdbg to add a breakpoint. Sends request to rdbg to delete a breakpoint. -- req_set_exception_breakpoints +- req_set_exception_breakpoints(breakpoints) -Sends request to rdbg to set exception breakpoints. +Sends request to rdbg to set exception breakpoints. e.g. + +```rb +req_set_exception_breakpoints([{ name: "RuntimeError", condition: "a == 1" }]) +``` + +Please note that `setExceptionBreakpoints` resets all exception breakpoints in every request. + +So the following code will only set breakpoint for `Exception`. + +```rb +req_set_exception_breakpoints([{ name: "RuntimeError" }]) +req_set_exception_breakpoints([{ name: "Exception" }]) +``` + +This means you can also use + +```rb +req_set_exception_breakpoints([]) +``` + +to clear all exception breakpoints. - req_continue diff --git a/test/protocol/catch_test.rb b/test/protocol/catch_test.rb index 185fed971..a1dd7f9c8 100644 --- a/test/protocol/catch_test.rb +++ b/test/protocol/catch_test.rb @@ -13,32 +13,31 @@ class CatchTest < TestCase 6| foo RUBY - def test_catch_stops_when_the_runtime_error_raised + def test_set_exception_breakpoints_sets_exception_breakpoints run_protocol_scenario PROGRAM do - req_set_exception_breakpoints + req_set_exception_breakpoints([{ name: "RuntimeError" }]) req_continue assert_line_num 3 req_terminate_debuggee end end - def test_set_exception_breakpoints_unset_exception_breakpoints + def test_set_exception_breakpoints_unsets_exception_breakpoints run_protocol_scenario PROGRAM, cdp: false do - req_set_exception_breakpoints - req_set_exception_breakpoints(exception: nil) + req_set_exception_breakpoints([{ name: "RuntimeError" }]) + req_set_exception_breakpoints([]) req_continue end end def test_set_exception_breakpoints_accepts_condition run_protocol_scenario PROGRAM, cdp: false do - req_set_exception_breakpoints(condition: "a == 2") + req_set_exception_breakpoints([{ name: "RuntimeError", condition: "a == 2" }]) req_continue - # exits directly because of error end run_protocol_scenario PROGRAM, cdp: false do - req_set_exception_breakpoints(condition: "a == 1") + req_set_exception_breakpoints([{ name: "RuntimeError", condition: "a == 1" }]) req_continue assert_line_num 3 req_terminate_debuggee diff --git a/test/support/protocol_utils.rb b/test/support/protocol_utils.rb index 45a188c15..391bd919f 100644 --- a/test/support/protocol_utils.rb +++ b/test/support/protocol_utils.rb @@ -125,15 +125,13 @@ def req_finish end end - def req_set_exception_breakpoints(exception: "RuntimeError", condition: nil) + def req_set_exception_breakpoints(breakpoints) case ENV['RUBY_DEBUG_TEST_UI'] when 'vscode' - filter_options = [] - - if exception - filter_option = { filterId: exception } - filter_option[:condition] = condition if condition - filter_options << filter_option + filter_options = breakpoints.map do |bp| + filter_option = { filterId: bp[:name] } + filter_option[:condition] = bp[:condition] if bp[:condition] + filter_option end send_dap_request 'setExceptionBreakpoints', filters: [], filterOptions: filter_options