don't send telemetry on expected errors

Author: lukaszsamson

apps/language_server/lib/language_server/dialyzer/analyzer.ex

@@ -1,6 +1,7 @@
 defmodule ElixirLS.LanguageServer.Dialyzer.Analyzer do
   require Record
   require Logger
+  alias ElixirLS.LanguageServer.JsonRpc
 
   # warn_race_condition is unsupported because it greatly increases analysis time
   # OTP 25 dropped support for warn_race_condition

apps/language_server/lib/language_server/providers/document_symbols.ex

@@ -32,7 +32,7 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbols do
         {:ok, build_symbols(symbols, uri, text, hierarchical)}
 
       {:error, :compilation_error} ->
-        {:error, :server_error, "[DocumentSymbols] Compilation error while parsing source file"}
+        {:error, :server_error, "Cannot parse source file", false}
     end
   end
 

apps/language_server/lib/language_server/providers/execute_command.ex

@@ -23,7 +23,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand do
       handler.execute(args, state)
     else
       _ ->
-        {:error, :invalid_request, nil}
+        {:error, :invalid_request, nil, true}
     end
   end
 

apps/language_server/lib/language_server/providers/execute_command/apply_spec.ex

@@ -90,7 +90,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ApplySpec do
 
       other ->
         {:error, :server_error,
-         "cannot insert spec, workspace/applyEdit returned #{inspect(other)}"}
+         "cannot insert spec, workspace/applyEdit returned #{inspect(other)}", true}
     end
   end
 end

apps/language_server/lib/language_server/providers/execute_command/get_ex_unit_tests_in_file.ex

@@ -7,8 +7,11 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.GetExUnitTestsInFile
     path = SourceFile.Path.from_uri(uri)
 
     case ExUnitTestTracer.get_tests(path) do
-      {:ok, tests} -> {:ok, tests}
-      {:error, reason} -> {:error, :server_error, inspect(reason)}
+      {:ok, tests} ->
+        {:ok, tests}
+
+      {:error, reason} ->
+        {:error, :server_error, "Cannot get tests in file: #{inspect(reason)}", true}
     end
   end
 end

apps/language_server/lib/language_server/providers/execute_command/manipulate_pipes.ex

@@ -52,17 +52,17 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ManipulatePipes do
       {:ok, nil}
     else
       {:error, :pipe_not_found} ->
-        {:error, :parse_error, "Pipe operator not found at cursor"}
+        {:error, :parse_error, "Pipe operator not found at cursor", false}
 
       {:error, :function_call_not_found} ->
-        {:error, :parse_error, "Function call not found at cursor"}
+        {:error, :parse_error, "Function call not found at cursor", false}
 
       {:error, :invalid_code} ->
-        {:error, :parse_error, "Malformed code"}
+        {:error, :parse_error, "Malformed code", false}
 
       error ->
         {:error, :server_error,
-         "Cannot execute pipe conversion, workspace/applyEdit returned #{inspect(error)}"}
+         "Cannot execute pipe conversion, workspace/applyEdit returned #{inspect(error)}", true}
     end
   end
 

apps/language_server/lib/language_server/providers/execute_command/mix_clean.ex

@@ -5,7 +5,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.MixClean do
   def execute([clean_deps?], _state) do
     case ElixirLS.LanguageServer.Build.clean(clean_deps?) do
       :ok -> {:ok, %{}}
-      {:error, reason} -> {:error, :server_error, inspect(reason)}
+      {:error, reason} -> {:error, :server_error, "Mix clean failed: #{inspect(reason)}", true}
     end
   end
 end

apps/language_server/lib/language_server/providers/folding_range.ex

@@ -84,15 +84,11 @@ defmodule ElixirLS.LanguageServer.Providers.FoldingRange do
       ]}
 
   """
-  @spec provide(%{text: String.t()}) :: {:ok, [t()]} | {:error, String.t()}
+  @spec provide(%{text: String.t()}) :: {:ok, [t()]}
   def provide(%{text: text}) do
     do_provide(text)
   end
 
-  def provide(not_a_source_file) do
-    {:error, "Expected a source file, found: #{inspect(not_a_source_file)}"}
-  end
-
   defp do_provide(text) do
     input = convert_text_to_input(text)
 

apps/language_server/lib/language_server/providers/folding_range/token.ex

@@ -6,6 +6,7 @@ defmodule ElixirLS.LanguageServer.Providers.FoldingRange.Token do
   """
 
   alias ElixirSense.Core.Normalized.Tokenizer
+  alias ElixirLS.LanguageServer.JsonRpc
   require Logger
 
   @type t :: {atom(), {non_neg_integer(), non_neg_integer(), any()}, any()}

apps/language_server/lib/language_server/providers/formatting.ex

@@ -15,14 +15,14 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
           end
 
         :error ->
-          {:error, :internal_error, "Unable to fetch formatter options"}
+          {:error, :internal_error, "Unable to fetch formatter options", true}
       end
     else
       msg =
         "Cannot format file from current directory " <>
           "(Currently in #{Path.relative_to(File.cwd!(), project_dir)})"
 
-      {:error, :internal_error, msg}
+      {:error, :internal_error, msg, true}
     end
   end
 
@@ -42,7 +42,7 @@ defmodule ElixirLS.LanguageServer.Providers.Formatting do
     {:ok, response}
   rescue
     _e in [TokenMissingError, SyntaxError] ->
-      {:error, :internal_error, "Unable to format due to syntax error"}
+      {:error, :internal_error, "Unable to format due to syntax error", false}
   end
 
   defp get_formatted(text, formatter, _) when is_function(formatter) do

apps/language_server/lib/language_server/server.ex

@@ -161,18 +161,20 @@ defmodule ElixirLS.LanguageServer.Server do
     {{_pid, command, start_time}, requests} = Map.pop!(state.requests, id)
 
     case result do
-      {:error, type, msg} ->
+      {:error, type, msg, send_telemetry} ->
         JsonRpc.respond_with_error(id, type, msg)
 
-        JsonRpc.telemetry(
-          "lsp_request_error",
-          %{
-            "elixir_ls.lsp_command" => String.replace(command, "/", "_"),
-            "elixir_ls.lsp_error" => type,
-            "elixir_ls.lsp_error_message" => msg
-          },
-          %{}
-        )
+        if send_telemetry do
+          JsonRpc.telemetry(
+            "lsp_request_error",
+            %{
+              "elixir_ls.lsp_command" => String.replace(command, "/", "_"),
+              "elixir_ls.lsp_error" => type,
+              "elixir_ls.lsp_error_message" => msg
+            },
+            %{}
+          )
+        end
 
       {:ok, result} ->
         elapsed = System.monotonic_time(:millisecond) - start_time
@@ -706,18 +708,20 @@ defmodule ElixirLS.LanguageServer.Server do
 
           state
 
-        {:error, type, msg, state} ->
+        {:error, type, msg, send_telemetry, state} ->
           JsonRpc.respond_with_error(id, type, msg)
 
-          JsonRpc.telemetry(
-            "lsp_request_error",
-            %{
-              "elixir_ls.lsp_command" => String.replace(command, "/", "_"),
-              "elixir_ls.lsp_error" => type,
-              "elixir_ls.lsp_error_message" => msg
-            },
-            %{}
-          )
+          if send_telemetry do
+            JsonRpc.telemetry(
+              "lsp_request_error",
+              %{
+                "elixir_ls.lsp_command" => String.replace(command, "/", "_"),
+                "elixir_ls.lsp_error" => type,
+                "elixir_ls.lsp_error_message" => msg
+              },
+              %{}
+            )
+          end
 
           state
 
@@ -999,10 +1003,10 @@ defmodule ElixirLS.LanguageServer.Server do
         {:ok, spec_code_lenses ++ test_code_lenses}
       else
         {:error, %ElixirSense.Core.Metadata{error: {line, error_msg}}} ->
-          {:error, :code_lens_error, "#{line}: #{error_msg}"}
+          {:error, :code_lens_error, "#{line}: #{error_msg}", true}
 
         {:error, error} ->
-          {:error, :code_lens_error, "Error while building code lenses: #{inspect(error)}"}
+          {:error, :code_lens_error, "Error while building code lenses: #{inspect(error)}", true}
 
         error ->
           error
@@ -1016,7 +1020,7 @@ defmodule ElixirLS.LanguageServer.Server do
     {:async,
      fn ->
        case ExecuteCommand.execute(command, args, state) do
-         {:error, :invalid_request, _msg} = res ->
+         {:error, :invalid_request, _msg, _} = res ->
            Logger.warning("Unmatched request: #{inspect(req)}")
            res
 
@@ -1027,24 +1031,19 @@ defmodule ElixirLS.LanguageServer.Server do
   end
 
   defp handle_request(folding_range_req(_id, uri), state = %__MODULE__{}) do
-    case get_source_file(state, uri) do
-      nil ->
-        {:error, :server_error, "Missing source file", state}
-
-      source_file ->
-        fun = fn -> FoldingRange.provide(source_file) end
-        {:async, fun, state}
-    end
+    source_file = get_source_file(state, uri)
+    fun = fn -> FoldingRange.provide(source_file) end
+    {:async, fun, state}
   end
 
   defp handle_request(%{"method" => "$/" <> _}, state = %__MODULE__{}) do
     # "$/" requests that the server doesn't support must return method_not_found
-    {:error, :method_not_found, nil, state}
+    {:error, :method_not_found, nil, false, state}
   end
 
   defp handle_request(req, state = %__MODULE__{}) do
     Logger.warning("Unmatched request: #{inspect(req)}")
-    {:error, :invalid_request, nil, state}
+    {:error, :invalid_request, nil, false, state}
   end
 
   defp handle_request_async(id, func) do
@@ -1056,7 +1055,7 @@ defmodule ElixirLS.LanguageServer.Server do
           func.()
         rescue
           e in InvalidParamError ->
-            {:error, :invalid_params, e.message}
+            {:error, :invalid_params, e.message, true}
         end
 
       GenServer.call(parent, {:request_finished, id, result}, :infinity)
@@ -1330,7 +1329,7 @@ defmodule ElixirLS.LanguageServer.Server do
 
       {:error, reason} ->
         if reason != :enoent do
-          Logger.warning("Couldn't read file #{file}: #{inspect(reason)}")
+          Logger.warning("Cannot read file #{file}: #{inspect(reason)}")
         end
 
         nil
@@ -1615,6 +1614,11 @@ defmodule ElixirLS.LanguageServer.Server do
       {:error, err} ->
         Logger.warning("Cannot create .elixir_ls/.gitignore, cause: #{Atom.to_string(err)}")
 
+        JsonRpc.show_message(
+          :warning,
+          "Cannot create .elixir_ls/.gitignore"
+        )
+
         state
     end
   end

apps/language_server/test/providers/document_symbols_test.exs

@@ -2465,8 +2465,8 @@ defmodule ElixirLS.LanguageServer.Providers.DocumentSymbolsTest do
     end
     """
 
-    assert {:error, :server_error, message} = DocumentSymbols.symbols(uri, text, true)
-    assert String.contains?(message, "Compilation error")
+    assert {:error, :server_error, message, false} = DocumentSymbols.symbols(uri, text, true)
+    assert String.contains?(message, "Cannot parse source file")
   end
 
   test "returns def and defp as a prefix" do

apps/language_server/test/providers/execute_command/get_ex_unit_tests_in_file_test.exs

@@ -51,7 +51,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.GetExUnitTestsInFileT
       in_fixture(Path.join(__DIR__, "../../../test_fixtures"), "project_with_tests", fn ->
         uri = SourceFile.Path.to_uri(Path.join(File.cwd!(), "test/error_test.exs"))
 
-        assert {:error, :server_error, "%TokenMissingError" <> _} =
+        assert {:error, :server_error, "Cannot get tests in file: %TokenMissingError" <> _, true} =
                  GetExUnitTestsInFile.execute([uri], nil)
       end)
     end

apps/language_server/test/providers/execute_command/manipulate_pipes_test.exs

@@ -577,7 +577,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ManipulatePipesTest d
 
       assert_never_raises(text, uri, "toPipe")
 
-      assert {:error, :parse_error, "Function call not found at cursor"} =
+      assert {:error, :parse_error, "Function call not found at cursor", false} =
                ManipulatePipes.execute(
                  ["toPipe", uri, 4, 13],
                  %Server{
@@ -1144,7 +1144,7 @@ defmodule ElixirLS.LanguageServer.Providers.ExecuteCommand.ManipulatePipesTest d
 
       assert_never_raises(text, uri, "fromPipe")
 
-      assert {:error, :parse_error, "Pipe operator not found at cursor"} =
+      assert {:error, :parse_error, "Pipe operator not found at cursor", false} =
                ManipulatePipes.execute(
                  ["fromPipe", uri, 4, 16],
                  %Server{

apps/language_server/test/providers/folding_range_test.exs

@@ -5,10 +5,6 @@ defmodule ElixirLS.LanguageServer.Providers.FoldingRangeTest do
 
   doctest(FoldingRange)
 
-  test "returns an :error tuple if input is not a source file" do
-    assert {:error, _} = %{} |> FoldingRange.provide()
-  end
-
   describe "indentation" do
     setup [:fold_via_indentation]
 

apps/language_server/test/providers/formatting_test.exs

@@ -269,7 +269,9 @@ defmodule ElixirLS.LanguageServer.Providers.FormattingTest do
 
       project_dir = maybe_convert_path_separators(FixtureHelpers.get_path("formatter"))
 
-      assert {:error, :internal_error, msg} = Formatting.format(source_file, uri, project_dir)
+      assert {:error, :internal_error, msg, false} =
+               Formatting.format(source_file, uri, project_dir)
+
       assert String.contains?(msg, "Unable to format")
     end)
   end
@@ -353,7 +355,9 @@ defmodule ElixirLS.LanguageServer.Providers.FormattingTest do
 
       project_dir = maybe_convert_path_separators(FixtureHelpers.get_path("formatter"))
 
-      assert {:error, :internal_error, msg} = Formatting.format(source_file, uri, project_dir)
+      assert {:error, :internal_error, msg, false} =
+               Formatting.format(source_file, uri, project_dir)
+
       assert String.contains?(msg, "Unable to format")
     end)
   end