From 0b407461b9bf985d61955eb6addc723a3ca3968c Mon Sep 17 00:00:00 2001
From: schneems <richard.schneeman+foo@gmail.com>
Date: Sat, 20 Nov 2021 15:58:38 -0600
Subject: [PATCH] Add explicit check for Syntax Error

As suggested in https://github.com/zombocom/dead_end/pull/119#discussion_r753592117 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 https://github.com/zombocom/dead_end/pull/114.
---
 lib/dead_end/api.rb                   |  9 +++++--
 lib/dead_end/pathname_from_message.rb |  2 +-
 spec/unit/api_spec.rb                 | 38 +++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/lib/dead_end/api.rb b/lib/dead_end/api.rb
index 8078b20..04edf07 100644
--- a/lib/dead_end/api.rb
+++ b/lib/dead_end/api.rb
@@ -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.
@@ -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
diff --git a/lib/dead_end/pathname_from_message.rb b/lib/dead_end/pathname_from_message.rb
index e3141eb..1ee9f53 100644
--- a/lib/dead_end/pathname_from_message.rb
+++ b/lib/dead_end/pathname_from_message.rb
@@ -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
 
diff --git a/spec/unit/api_spec.rb b/spec/unit/api_spec.rb
index de8f746..e714b58 100644
--- a/spec/unit/api_spec.rb
+++ b/spec/unit/api_spec.rb
@@ -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,
@@ -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