Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rb] Add backtrace locations and cause to errors #14170

Merged
merged 42 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
3ff3559
Add backtrace locations and cause to errors
aguspe Jun 22, 2024
eb6aa00
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 22, 2024
ec6f5bc
Add type support and save value to variable
aguspe Jun 22, 2024
6540f73
Merge branch 'fix_backtrace_location' of github.com:aguspe/selenium i…
aguspe Jun 22, 2024
777a8a6
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 22, 2024
6883b0a
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 22, 2024
6a76481
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 22, 2024
0447c6d
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 23, 2024
4aae5a3
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 24, 2024
7e4a2cf
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 24, 2024
3c7b386
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 24, 2024
c4e2593
Resolve conflicts and update the error class
aguspe Jun 25, 2024
de16eec
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 26, 2024
4afe312
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 26, 2024
33bd311
Update implementation
aguspe Jun 26, 2024
48cb412
Merge branch 'fix_backtrace_location' of github.com:aguspe/selenium i…
aguspe Jun 26, 2024
10791b2
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 26, 2024
5139bbf
Remove unused methods
aguspe Jun 26, 2024
d798475
Merge branch 'fix_backtrace_location' of github.com:aguspe/selenium i…
aguspe Jun 26, 2024
2996653
Error has cause, backtrace and backtrace locations
aguspe Jun 27, 2024
05cfce6
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 27, 2024
ada5081
Remove english
aguspe Jun 27, 2024
7529157
Merge branch 'fix_backtrace_location' of github.com:aguspe/selenium i…
aguspe Jun 27, 2024
3c64035
Remove unnecessary attributes
aguspe Jun 27, 2024
5c9808a
Return ex
aguspe Jun 27, 2024
176286c
Fix line issue
aguspe Jun 27, 2024
48a7105
Add backtrace back
aguspe Jun 27, 2024
ef832d5
Fix types
aguspe Jun 27, 2024
1f1dcae
Add caller
aguspe Jun 27, 2024
c36c565
Merge branch 'trunk' into fix_backtrace_location
aguspe Jun 28, 2024
7497293
Address review comment
aguspe Jun 28, 2024
2f8b9f7
Include backtrace on the cause
aguspe Jun 29, 2024
a506504
Update types
aguspe Jun 29, 2024
c7bc5d2
Merge branch 'trunk' into fix_backtrace_location
aguspe Jul 5, 2024
32536f5
Merge branch 'trunk' into fix_backtrace_location
aguspe Jul 7, 2024
eb0248e
Merge branch 'trunk' into fix_backtrace_location
aguspe Jul 11, 2024
9ff8252
Merge branch 'trunk' into fix_backtrace_location
aguspe Jul 12, 2024
ea9f4b4
Merge branch 'trunk' into fix_backtrace_location
aguspe Jul 13, 2024
ac8e22a
Improve error types
aguspe Jul 13, 2024
bfedee9
Improve error types
aguspe Jul 13, 2024
834a678
Improve add cause
aguspe Jul 16, 2024
0ef5372
Roll back backtrace on message
aguspe Jul 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions rb/lib/selenium/webdriver/common/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,7 @@ class UnknownCommandError < WebDriverError; end
# A command failed because the referenced element is no longer attached to the DOM.
#

class StaleElementReferenceError < WebDriverError
def initialize(msg = '')
super("#{msg}; #{SUPPORT_MSG} #{URLS[:StaleElementReferenceError]}")
end
end
class StaleElementReferenceError < WebDriverError; end
aguspe marked this conversation as resolved.
Show resolved Hide resolved

#
# A command failed because the referenced shadow root is no longer attached to the DOM.
Expand Down
41 changes: 8 additions & 33 deletions rb/lib/selenium/webdriver/remote/response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Response
attr_reader :code, :payload

def initialize(code, payload = nil)
@code = code
@code = code
@payload = payload || {}

assert_ok
Expand All @@ -37,11 +37,8 @@ def initialize(code, payload = nil)
def error
error, message, backtrace = process_error
klass = Error.for_error(error) || return

ex = klass.new(message)
ex.set_backtrace(caller)
add_backtrace ex, backtrace

add_cause(ex, error, backtrace)
ex
end

Expand All @@ -59,34 +56,12 @@ def assert_ok
raise Error::ServerError, self
end

def add_backtrace(ex, server_trace)
aguspe marked this conversation as resolved.
Show resolved Hide resolved
return unless server_trace

backtrace = case server_trace
when Array
backtrace_from_remote(server_trace)
when String
server_trace.split("\n")
end

ex.set_backtrace(backtrace + ex.backtrace)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the only line that should have been changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed during testing that when setting the backtrace to the exception a couple of things happened:

  • The cause was not set which was the issue

  • Depending on the type of ex.backtrace this ended up in a type error

  • As the example in the previous conversation and the testing done with the selenium grid, it did not seem to have the effect we expected in the formatting of the backtrace, I added examples and my reasoning here: [rb] Add backtrace locations and cause to errors #14170 (comment)

Let me know if this doesn't make much sense to you and I will gladly make a PR to update this :) thank you so much for the help!

end

def backtrace_from_remote(server_trace)
aguspe marked this conversation as resolved.
Show resolved Hide resolved
server_trace.filter_map do |frame|
next unless frame.is_a?(Hash)

file = frame['fileName']
line = frame['lineNumber']
meth = frame['methodName']

class_name = frame['className']
file = "#{class_name}(#{file})" if class_name

meth = 'unknown' if meth.nil? || meth.empty?

"[remote server] #{file}:#{line}:in `#{meth}'"
end
def add_cause(ex, error, backtrace)
cause = Error::WebDriverError.new
cause.set_backtrace(backtrace)
raise ex, cause: cause
rescue Error.for_error(error)
ex
end

def process_error
Expand Down
8 changes: 3 additions & 5 deletions rb/sig/lib/selenium/webdriver/common/error.rbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Selenium
module WebDriver
module Error
def self.for_error: (untyped error) -> untyped
def self.for_error: (String? error) -> Class?

SUPPORT_MSG: String

Expand All @@ -10,11 +10,12 @@ module Selenium
URLS: Hash[Symbol, String]

class WebDriverError < StandardError
def initialize: (?String | Array[String] msg) -> void

def class_name: -> Symbol?
end

class NoSuchElementError < WebDriverError
def initialize: (?String msg) -> void
end

class NoSuchFrameError < WebDriverError
Expand All @@ -24,7 +25,6 @@ module Selenium
end

class StaleElementReferenceError < WebDriverError
def initialize: (?String msg) -> void
end

class DetachedShadowRootError < WebDriverError
Expand Down Expand Up @@ -61,7 +61,6 @@ module Selenium
end

class InvalidSelectorError < WebDriverError
def initialize: (?::String msg) -> void
end

class SessionNotCreatedError < WebDriverError
Expand Down Expand Up @@ -101,7 +100,6 @@ module Selenium
end

class NoSuchDriverError < WebDriverError
def initialize: (?String msg) -> void
end
end
end
Expand Down
10 changes: 4 additions & 6 deletions rb/sig/lib/selenium/webdriver/remote/response.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,17 @@ module Selenium

def initialize: (String code, ?Hash[untyped, untyped]? payload) -> void

def error: () -> (nil | untyped)
def error: () -> Error::WebDriverError?

def []: (untyped key) -> untyped

private

def assert_ok: () -> (nil | untyped)
def assert_ok: () -> Error::WebDriverError

def add_backtrace: (untyped ex, untyped server_trace) -> (nil | untyped)
def add_cause: (Error::WebDriverError ex, String error, Array[String] backtrace) -> Error::WebDriverError

def backtrace_from_remote: (untyped server_trace) -> String

def process_error: () -> (nil | untyped)
def process_error: () -> Array[Hash[untyped, untyped]]
end
end
end
Expand Down
18 changes: 18 additions & 0 deletions rb/spec/integration/selenium/webdriver/error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,24 @@ module WebDriver
driver.find_element(id: 'nonexistent')
}.to raise_error(WebDriver::Error::NoSuchElementError, /#no-such-element-exception/)
end

it 'has backtrace locations' do
driver.find_element(id: 'nonexistent')
rescue WebDriver::Error::NoSuchElementError => e
expect(e.backtrace_locations).not_to be_empty
end

it 'has cause' do
driver.find_element(id: 'nonexistent')
rescue WebDriver::Error::NoSuchElementError => e
expect(e.cause).to be_a(WebDriver::Error::WebDriverError)
end

it 'has backtrace' do
driver.find_element(id: 'nonexistent')
rescue WebDriver::Error::NoSuchElementError => e
expect(e.backtrace).not_to be_empty
end
end
end # WebDriver
end # Selenium
Loading