fix: prevent RuntimeError missing Absinthe.Phase.Document.Result for custom :result_phase (OPS-3643)

CHANGELOG.md

@@ -1,3 +1,9 @@
+v1.4.6
+
+- Bug fix: Prevent RuntimeError "Could not find phase Absinthe.Phase.Document.Result" when using custom `:result_phase` (directly via `Absinthe.Plug.init/1` or via `:pipeline` override that does `Absinthe.Pipeline.replace/3` on the Result phase) in combination with error jumps from Absinthe phases (e.g. complexity or validation errors). The carried `:result_phase` in pipeline options is now synced to the actual result phase present in the final pipeline, and `default_pipeline/2` natively supports and replaces for `:result_phase`.
+- Chore: Loosen plug dep constraint and upgrade plug, poison, transitive (mime etc) within ranges as part of keeping dependencies fresh.
+- Add support for `:result_phase` option (analogous to analyze/max_complexity).
+
 v1.4.5
 
 - Fixes support for newer Phoenix versions.

lib/absinthe/plug.ex

@@ -124,7 +124,7 @@ defmodule Absinthe.Plug do
 
   alias __MODULE__.Request
 
-  @raw_options [:analyze_complexity, :max_complexity]
+  @raw_options [:analyze_complexity, :max_complexity, :result_phase]
 
   @type function_name :: atom
 
@@ -142,7 +142,8 @@ defmodule Absinthe.Plug do
   - `:log_level` -- (Optional) Set the logger level for Absinthe Logger. Defaults to `:debug`.
   - `:analyze_complexity` -- (Optional) Set whether to calculate the complexity of incoming GraphQL queries.
   - `:max_complexity` -- (Optional) Set the maximum allowed complexity of the GraphQL query. If a document’s calculated complexity exceeds the maximum, resolution will be skipped and an error will be returned in the result detailing the calculated and maximum complexities.
-
+  - `:result_phase` -- (Optional) The phase to use to produce the final result (default: `Absinthe.Phase.Document.Result`). Use to customize result formatting (e.g. order preservation).
+
   """
   @type opts :: [
     schema: module,
@@ -154,6 +155,7 @@ defmodule Absinthe.Plug do
     document_providers: [Absinthe.Plug.DocumentProvider.t, ...] | Absinthe.Plug.DocumentProvider.t | {module, atom},
     analyze_complexity: boolean,
     max_complexity: non_neg_integer | :infinity,
+    result_phase: module,
     serializer: module | {module, Keyword.t},
     content_type: String.t,
     before_send: {module, atom},
@@ -440,13 +442,49 @@ defmodule Absinthe.Plug do
   """
   @spec default_pipeline(map, Keyword.t) :: Absinthe.Pipeline.t
   def default_pipeline(config, pipeline_opts) do
+    result_phase = Keyword.get(pipeline_opts, :result_phase, Absinthe.Phase.Document.Result)
     config.schema_mod
     |> Absinthe.Pipeline.for_document(pipeline_opts)
     |> Absinthe.Pipeline.insert_after(Absinthe.Phase.Document.CurrentOperation,
       [
         {Absinthe.Plug.Validation.HTTPMethod, method: config.conn_private.http_method},
       ]
     )
+    |> replace_result_phase(result_phase)
+  end
+
+  defp replace_result_phase(pipeline, Absinthe.Phase.Document.Result), do: pipeline
+  defp replace_result_phase(pipeline, phase) do
+    Absinthe.Pipeline.replace(pipeline, Absinthe.Phase.Document.Result, phase)
+  end
+
+  defp find_actual_result_phase(pipeline) do
+    pipeline
+    |> List.flatten()
+    |> Enum.reverse()
+    |> Enum.find_value(fn
+      {phase, _} -> if not telemetry_phase?(phase), do: phase
+      phase -> if not telemetry_phase?(phase), do: phase
+    end)
+    || Absinthe.Phase.Document.Result
+  end
+
+  defp telemetry_phase?(Absinthe.Phase.Telemetry), do: true
+  defp telemetry_phase?({Absinthe.Phase.Telemetry, _}), do: true
+  defp telemetry_phase?(_), do: false
+
+  def ensure_result_phase_consistent(pipeline) do
+    actual = find_actual_result_phase(pipeline)
+    if actual == Absinthe.Phase.Document.Result do
+      pipeline
+    else
+      Enum.map(pipeline, fn
+        {phase, opts} when is_list(opts) ->
+          opts = if Keyword.has_key?(opts, :result_phase), do: Keyword.put(opts, :result_phase, actual), else: opts
+          {phase, opts}
+        other -> other
+      end)
+    end
   end
 
   #

lib/absinthe/plug/request/query.ex

@@ -77,6 +77,7 @@ defmodule Absinthe.Plug.Request.Query do
     pipeline =
       %{query | pipeline: pipeline}
       |> Absinthe.Plug.DocumentProvider.pipeline
+      |> Absinthe.Plug.ensure_result_phase_consistent
 
     %{query | pipeline: pipeline}
   end

mix.exs

@@ -1,7 +1,7 @@
 defmodule Absinthe.Plug.Mixfile do
   use Mix.Project
 
-  @version "1.4.5"
+  @version "1.4.6"
 
   def project do
     [
@@ -41,7 +41,7 @@ defmodule Absinthe.Plug.Mixfile do
 
   defp deps do
     [
-      {:plug, "~> 1.3.2 or ~> 1.4"},
+      {:plug, ">= 1.3.2 and < 2.0.0"},
       {:absinthe, "~> 1.4.11"},
       {:poison, ">= 0.0.0", only: [:dev, :test]},
       {:ex_doc, "~> 0.18.0", only: :dev}

mix.lock

@@ -1,8 +1,10 @@
 %{
-  "absinthe": {:hex, :absinthe, "1.4.13", "81eb2ff41f1b62cd6e992955f62c22c042d1079b7936c27f5f7c2c806b8fc436", [:mix], [{:dataloader, "~> 1.0.0", [hex: :dataloader, repo: "hexpm", optional: true]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"},
-  "earmark": {:hex, :earmark, "1.2.5", "4d21980d5d2862a2e13ec3c49ad9ad783ffc7ca5769cf6ff891a4553fbaae761", [:mix], [], "hexpm"},
-  "ex_doc": {:hex, :ex_doc, "0.18.3", "f4b0e4a2ec6f333dccf761838a4b253d75e11f714b85ae271c9ae361367897b7", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
-  "mime": {:hex, :mime, "1.2.0", "78adaa84832b3680de06f88f0997e3ead3b451a440d183d688085be2d709b534", [:mix], [], "hexpm"},
-  "plug": {:hex, :plug, "1.5.0", "224b25b4039bedc1eac149fb52ed456770b9678bbf0349cdd810460e1e09195b", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1 or ~> 2.1", [hex: :cowboy, repo: "hexpm", optional: true]}, {:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}], "hexpm"},
-  "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
+  "absinthe": {:hex, :absinthe, "1.4.13", "81eb2ff41f1b62cd6e992955f62c22c042d1079b7936c27f5f7c2c806b8fc436", [:mix], [{:dataloader, "~> 1.0.0", [hex: :dataloader, repo: "hexpm", optional: true]}, {:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "ddb55fba830d5a25add2d707a81d11b40096bf6f2cd43b95e117610e7bb1df1a"},
+  "earmark": {:hex, :earmark, "1.2.5", "4d21980d5d2862a2e13ec3c49ad9ad783ffc7ca5769cf6ff891a4553fbaae761", [:mix], [], "hexpm", "c57508ddad47dfb8038ca6de1e616e66e9b87313220ac5d9817bc4a4dc2257b9"},
+  "ex_doc": {:hex, :ex_doc, "0.18.3", "f4b0e4a2ec6f333dccf761838a4b253d75e11f714b85ae271c9ae361367897b7", [:mix], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm", "33d7b70d87d45ed899180fb0413fb77c7c48843188516e15747e00fdecf572b6"},
+  "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"},
+  "plug": {:hex, :plug, "1.19.2", "e4950525b22c6789dfb38a3f95d47171ba159da3fc5a33be9643b43d5e8adb98", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b6fce20a56af5e60fa5dfecf3f907bb98ec981be43c79a3809a499bc3d133de0"},
+  "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"},
+  "poison": {:hex, :poison, "6.0.0", "9bbe86722355e36ffb62c51a552719534257ba53f3271dacd20fbbd6621a583a", [:mix], [{:decimal, "~> 2.1", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "bb9064632b94775a3964642d6a78281c07b7be1319e0016e1643790704e739a2"},
+  "telemetry": {:hex, :telemetry, "1.4.2", "a0cb522801dffb1c49fe6e30561badffc7b6d0e180db1300df759faa22062855", [:rebar3], [], "hexpm", "928f6495066506077862c0d1646609eed891a4326bee3126ba54b60af61febb1"},
 }

test/lib/absinthe/plug_test.exs

@@ -1,3 +1,10 @@
+defmodule Absinthe.PlugTest.CustomResult do
+  use Absinthe.Phase
+  def run(bp, opts) do
+    Absinthe.Phase.Document.Result.run(bp, opts)
+  end
+end
+
 defmodule Absinthe.PlugTest do
   use Absinthe.Plug.TestCase
   alias Absinthe.Plug.TestSchema
@@ -175,6 +182,38 @@ defmodule Absinthe.PlugTest do
     assert %{"errors" => [%{"message" => "Field complex is too complex" <> _} | _]} = resp_body |> Poison.decode!
   end
 
+  test "custom result phase via pipeline override works with jumps (no missing Result phase)" do
+    opts = Absinthe.Plug.init(
+      schema: TestSchema,
+      analyze_complexity: true,
+      max_complexity: 99,
+      pipeline: {__MODULE__, :pipeline_with_custom_result}
+    )
+
+    assert %{status: 200, resp_body: resp_body} = conn(:get, "/", query: @complex_query)
+    |> put_req_header("content-type", "application/x-www-form-urlencoded")
+    |> plug_parser
+    |> Absinthe.Plug.call(opts)
+
+    assert %{"errors" => [%{"message" => "Field complex is too complex" <> _} | _]} = resp_body |> Poison.decode!
+  end
+
+  test "result_phase option passed to init is honored for custom result phase" do
+    opts = Absinthe.Plug.init(
+      schema: TestSchema,
+      analyze_complexity: true,
+      max_complexity: 99,
+      result_phase: Absinthe.PlugTest.CustomResult
+    )
+
+    assert %{status: 200, resp_body: resp_body} = conn(:get, "/", query: @complex_query)
+    |> put_req_header("content-type", "application/x-www-form-urlencoded")
+    |> plug_parser
+    |> Absinthe.Plug.call(opts)
+
+    assert %{"errors" => [%{"message" => "Field complex is too complex" <> _} | _]} = resp_body |> Poison.decode!
+  end
+
   @query """
   {
     item(id: "foo") {
@@ -458,4 +497,10 @@ defmodule Absinthe.PlugTest do
     Map.put(context, :opts, Absinthe.Plug.init(schema: TestSchema))
   end
 
+  def pipeline_with_custom_result(config, opts) do
+    config
+    |> Absinthe.Plug.default_pipeline(opts)
+    |> Absinthe.Pipeline.replace(Absinthe.Phase.Document.Result, Absinthe.PlugTest.CustomResult)
+  end
+
 end