extract_sources works with Windows paths #91
Conversation
Prior to this backtrace lines were simply split by a single colon. Unfortunately that is also the drive letter delimiter in Windows paths which resulted in a lot of empty source fragments of "C:0". Now the trace line is split by the first colon followed by some digits, which works for both Windows and Unix path styles.
| @@ -24,7 +24,7 @@ def traces | |||
|
|
|||
| def extract_sources | |||
| exception.backtrace.map do |trace| | |||
| file, line = trace.split(":") | |||
| file, line = trace.match(/^(.+?):(\d+).*$/).captures | |||
There was a problem hiding this comment.
It is common to have custom backtraces. Take IRB's "(irb)" caller, for example. This won't catch it, will return nil and the captures call will raise a NoMethodError. Can we handle this case as well?
There was a problem hiding this comment.
You are right, that would be a good precaution. I'm on it.
BTW: How do you produce a bare "(irb)" as a trace line? All I could provoke was "(irb):1" (i.e. with a colon and a number, which is matched by the pattern)
There was a problem hiding this comment.
On the other hand: The Ruby documentation states that a line number is always present (http://www.ruby-doc.org/core-2.1.5/Exception.html#method-i-backtrace) and demands that manually added trace lines must adhere to the specified format (http://www.ruby-doc.org/core-2.1.5/Exception.html#method-i-set_backtrace)
There was a problem hiding this comment.
>> exc = RuntimeError.new
=> #<RuntimeError: RuntimeError>
>> exc.set_backtrace("whadup")
=> ["whadup"]
>> raise exc
RuntimeError: RuntimeError
from whadupThere was a problem hiding this comment.
Ah, ok, I was looking for a "normal" way to produce such a backtrace. Brute-forcing it didn't cross my mind :)
The previous commit would result in a runtime error if a backtrace line didn't conform to the required format.
|
@daniel-rikowski Thank you for the work so far! I'm really sorry to bother you with this, but I think we should move the issue against upstream Rails. The file you wanna look for is actionpack/lib/action_dispatch/middleware/exception_wrapper.rb The reason for this is that we used to monkey patch and/or duplicate upstream Rails code in this project. We fixed this in #69, so its easier to fix problems present both in Rails and web-console. We can even get a better review at the Rails tracker and you can put your name on the front page of contributors.rubyonrails.org. Again, thank you so much for working on this issue! |
This is an issue brought up by @daniel-rikowski in rails/web-console#91. Citing his PR proposal here: > Prior to this, backtrace lines were simply split by a single colon. > > Unfortunately that is also the drive letter delimiter in Windows paths > which resulted in a lot of empty source fragments of "C:0". ("C" from > the drive letter and 0 from "/path/to/rails/file.rb:16".to_i) > > Now the trace line is split by the first colon followed by some digits, > which works for both Windows and Unix path styles. Now, the PR was sent against web-console, because of the templates copy issue we used to had. Instead of bothering the contributor to reopen the issue against upstream Rails itself, I will make sure he gets the credit by putting his name in [rails-contributors/hard_coded_authors.rb][]. [rails-contributors/hard_coded_authors.rb]: (https://github.com/fxn/rails-contributors/blob/master/app/models/names_manager/hard_coded_authors.rb).
Prior to this, backtrace lines were simply split by a single colon.
Unfortunately that is also the drive letter delimiter in Windows paths which resulted in a lot of empty source fragments of "C:0". ("C" from the drive letter and 0 from
"/path/to/rails/file.rb:16".to_i)Now the trace line is split by the first colon followed by some digits, which works for both Windows and Unix path styles.