Skip to content

Commit

Permalink
Add explicit check for Syntax Error
Browse files Browse the repository at this point in the history
As suggested in #119 (comment) we are now explicitly checking the type of the incoming error. Rather than raise a new error, I've chosen to emit a warning and re-raise the original error. This is a similar pattern to the case where we are not able to detect the filename from the error message. The goal is to provide visibility into the source of the behavior, but to break as few expectations as possible. 

The `DeadEnd.handle_error` should (ideally) never raise an error to the end user. The purpose of the library is to provide progressive error enhancement. If it fails it means we cannot make a better error, so therefore we should warn (so the user can fix or report the problem) and then fall back to the original error.

I also added a test for the failure error condition where the filename cannot be pulled out of the SyntaxError message which was added in #114.
  • Loading branch information
schneems committed Nov 20, 2021
1 parent 78297ee commit 0b40746
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 3 deletions.
9 changes: 7 additions & 2 deletions lib/dead_end/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Error < StandardError; end

# DeadEnd.handle_error [Public]
#
# Takes an exception from a syntax error, uses that
# Takes a `SyntaxError`` exception, uses the
# error message to locate the file. Then the file
# will be analyzed to find the location of the syntax
# error and emit that location to stderr.
Expand All @@ -37,7 +37,12 @@ class Error < StandardError; end
# exception will be re-raised (even with
# `re_raise: false`).
def self.handle_error(e, re_raise: true, io: $stderr)
file = PathnameFromMessage.new(e.message).call.name
unless e.is_a?(SyntaxError)
io.puts("DeadEnd: Must pass a SyntaxError, got: #{e.class}")
raise e
end

file = PathnameFromMessage.new(e.message, io: io).call.name
raise e unless file

io.sync = true
Expand Down
2 changes: 1 addition & 1 deletion lib/dead_end/pathname_from_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def call
end

if @parts.empty?
@io.puts "DeadEnd: could not find filename from #{@line.inspect}"
@io.puts "DeadEnd: Could not find filename from #{@line.inspect}"
@name = nil
end

Expand Down
38 changes: 38 additions & 0 deletions spec/unit/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ def fake_error.message
"#{__FILE__}:216: unterminated string meets end of file "
end

def fake_error.is_a?(v)
true
end

io = StringIO.new
DeadEnd.handle_error(
fake_error,
Expand All @@ -20,5 +24,39 @@ def fake_error.message

expect(io.string.strip).to eq("Syntax OK")
end

it "raises original error with warning if a non-syntax error is passed" do
error = NameError.new("blerg")
io = StringIO.new
expect {
DeadEnd.handle_error(
error,
re_raise: false,
io: io
)
}.to raise_error { |e|
expect(io.string).to include("Must pass a SyntaxError")
expect(e).to eq(error)
}
end

it "raises original error with warning if file is not found" do
fake_error = SyntaxError.new
def fake_error.message
"#does/not/exist/lol/doesnotexist:216: unterminated string meets end of file "
end

io = StringIO.new
expect {
DeadEnd.handle_error(
fake_error,
re_raise: false,
io: io
)
}.to raise_error { |e|
expect(io.string).to include("Could not find filename")
expect(e).to eq(fake_error)
}
end
end
end

0 comments on commit 0b40746

Please sign in to comment.