fix(core): stop silently shortening gRPC deadlines on the wire

grpc/test/grpc/adapters/mint_test.exs

@@ -181,10 +181,10 @@ defmodule GRPC.Client.Adapters.MintTest do
       refute Process.alive?(stream_response_pid)
     end
 
-    test "accepts the float milliseconds a :deadline is resolved into", %{stream: stream} do
+    test "accepts the milliseconds a :deadline is resolved into", %{stream: stream} do
       timeout = GRPC.TimeUtils.to_relative(DateTime.add(DateTime.utc_now(), 20, :millisecond))
 
-      assert is_float(timeout)
+      assert is_number(timeout)
 
       assert {:error, %GRPC.RPCError{status: status}} =
                Mint.receive_data(stream, timeout: timeout)

grpc_core/lib/grpc/time_utils.ex

@@ -2,23 +2,17 @@ defmodule GRPC.TimeUtils do
   @moduledoc false
 
   @doc """
-  Returns relative time in milliseconds.
+  Returns relative time in whole milliseconds, truncated so a deadline is never extended.
 
   ## Examples
 
       iex> from = DateTime.utc_now
       iex> us = DateTime.to_unix(from, :microsecond)
       iex> datetime = DateTime.from_unix!(us + 5005, :microsecond)
-      iex> Float.round(GRPC.TimeUtils.to_relative(datetime, from), 3)
-      5.005
+      iex> GRPC.TimeUtils.to_relative(datetime, from)
+      5
   """
   def to_relative(datetime, from \\ DateTime.utc_now()) do
-    ms = datetime_to_milliseconds(datetime)
-    now_ms = datetime_to_milliseconds(from)
-    ms - now_ms
-  end
-
-  defp datetime_to_milliseconds(datetime) do
-    DateTime.to_unix(datetime, :second) * 1000 + elem(datetime.microsecond, 0) * 0.001
+    datetime |> DateTime.diff(from, :microsecond) |> div(1000)
   end
 end

grpc_core/lib/grpc/transport/utils.ex

@@ -7,8 +7,9 @@ defmodule GRPC.Transport.Utils do
   # @ms_ceiling @us_ceiling * 1000
 
   # unit: ms
-  @ms_ceiling 1000
-  @second_ceiling @ms_ceiling * 60
+  # TimeoutValue is capped at 8 digits, so anything below @ms_ceiling encodes exactly.
+  @ms_ceiling 100_000_000
+  @second_ceiling @ms_ceiling * 1000
   @minute_ceiling @second_ceiling * 60
 
   @doc """

grpc_core/test/grpc/time_utils_test.exs

@@ -2,4 +2,29 @@ defmodule GRPC.TimeUtilsTest do
   use ExUnit.Case, async: true
 
   doctest GRPC.TimeUtils
+
+  describe "to_relative/2" do
+    test "returns an integer, because append_timeout/2 drops a float rather than sending it" do
+      from = DateTime.utc_now()
+
+      for offset_us <- [1_000, 5_005, 2_000_000, 999] do
+        result = GRPC.TimeUtils.to_relative(DateTime.add(from, offset_us, :microsecond), from)
+        assert is_integer(result), "got #{inspect(result)} for #{offset_us}us"
+      end
+    end
+
+    test "truncates rather than rounds, so a deadline is never extended" do
+      from = DateTime.utc_now()
+      almost_6ms = DateTime.add(from, 5_999, :microsecond)
+
+      assert GRPC.TimeUtils.to_relative(almost_6ms, from) == 5
+    end
+
+    test "an already-expired deadline is non-positive" do
+      from = DateTime.utc_now()
+      past = DateTime.add(from, -1_500, :millisecond)
+
+      assert GRPC.TimeUtils.to_relative(past, from) <= 0
+    end
+  end
 end

grpc_core/test/grpc/transport/http2_timeout_test.exs

@@ -0,0 +1,31 @@
+defmodule GRPC.Transport.HTTP2TimeoutTest do
+  use ExUnit.Case, async: true
+
+  alias GRPC.Transport.HTTP2
+
+  @stream %{
+    codec: GRPC.Codec.Proto,
+    compressor: nil,
+    accepted_compressors: [],
+    channel: %{headers: %{}},
+    headers: %{}
+  }
+
+  defp timeout_header(opts) do
+    HTTP2.client_headers_without_reserved(@stream, opts)
+    |> Enum.find(fn {k, _v} -> k == "grpc-timeout" end)
+  end
+
+  describe "grpc-timeout header" do
+    test "an integer timeout is sent in milliseconds" do
+      assert timeout_header(%{timeout: 5}) == {"grpc-timeout", "5m"}
+      assert timeout_header(%{timeout: 1500}) == {"grpc-timeout", "1500m"}
+    end
+
+    test ":infinity and nil send no deadline" do
+      assert timeout_header(%{timeout: :infinity}) == nil
+      assert timeout_header(%{timeout: nil}) == nil
+      assert timeout_header(%{}) == nil
+    end
+  end
+end

grpc_core/test/grpc/transport/utils_test.exs

@@ -0,0 +1,113 @@
+defmodule GRPC.Transport.UtilsTest do
+  use ExUnit.Case, async: true
+
+  import GRPC.Transport.Utils
+
+  # unit: ns
+  @ns_ceiling 1000
+  @us_ceiling 1000_000
+
+  # unit: ms
+  @ms_ceiling 100_000_000
+  @second_ceiling @ms_ceiling * 1000
+  @minute_ceiling @second_ceiling * 60
+
+  test "encode_ns/1 returns 0" do
+    assert encode_ns(-1) == "0u"
+    assert encode_ns(0) == "0u"
+  end
+
+  test "encode_ns/1 returns nanoseconds" do
+    assert encode_ns(1) == "1n"
+    assert encode_ns(@ns_ceiling - 1) == "999n"
+  end
+
+  test "encode_ns/1 returns microseconds" do
+    assert encode_ns(@ns_ceiling) == "1u"
+    assert encode_ns(@us_ceiling - 1) == "999u"
+  end
+
+  test "encode_timeout/1 returns 0" do
+    assert encode_timeout(-1) == "0u"
+    assert encode_timeout(0) == "0u"
+  end
+
+  test "encode_timeout/1 returns millisecond" do
+    assert encode_timeout(1) == "1m"
+    assert encode_timeout(1500) == "1500m"
+    assert encode_timeout(@ms_ceiling - 1) == "99999999m"
+  end
+
+  test "encode_timeout/1 returns second" do
+    assert encode_timeout(@ms_ceiling) == "100000S"
+    assert encode_timeout(@second_ceiling - 1) == "99999999S"
+  end
+
+  test "encode_timeout/1 returns minute" do
+    assert encode_timeout(@second_ceiling) == "1666666M"
+    assert encode_timeout(@minute_ceiling - 1) == "99999999M"
+  end
+
+  test "encode_timeout/1 returns hour" do
+    assert encode_timeout(@minute_ceiling) == "1666666H"
+  end
+
+  describe "encode_timeout/1 fidelity" do
+    test "millisecond values survive a round-trip exactly" do
+      for ms <- [1, 999, 1000, 1500, 2000, 2500, 3847, 5000, 59_999, 60_000, 3_600_000] do
+        assert decode_timeout(encode_timeout(ms)) == ms,
+               "#{ms} ms did not survive encode/decode: " <>
+                 "#{inspect(encode_timeout(ms))} -> #{decode_timeout(encode_timeout(ms))} ms"
+      end
+    end
+
+    test "values above the millisecond ceiling lose less than one second" do
+      ms = @ms_ceiling + 1
+      decoded = decode_timeout(encode_timeout(ms))
+
+      assert decoded <= ms
+      assert ms - decoded < 1000
+    end
+
+    test "the encoded value stays within the 8-digit wire limit" do
+      for ms <- [
+            1,
+            @ms_ceiling - 1,
+            @ms_ceiling,
+            @second_ceiling - 1,
+            @second_ceiling,
+            @minute_ceiling - 1,
+            @minute_ceiling
+          ] do
+        {digits, _unit} = String.split_at(encode_timeout(ms), -1)
+
+        assert String.length(digits) <= 8,
+               "#{ms} ms encoded to #{digits} (#{String.length(digits)} digits)"
+      end
+    end
+  end
+
+  test "decode_timeout/1 returns 0" do
+    assert decode_timeout("0u") == 0
+  end
+
+  test "decode_timeout/1 returns 0.123" do
+    assert decode_timeout("123u") == 0
+  end
+
+  test "decode_timeout/1 returns 123 ms" do
+    assert decode_timeout("123m") == 123
+  end
+
+  test "decode_timeout/1 returns seconds" do
+    assert decode_timeout("123S") == 123_000
+  end
+
+  test "decode_timeout/1 returns minutes" do
+    assert decode_timeout("123M") == 123 * 60_000
+  end
+
+  test "decode_timeout/1 returns hour" do
+    assert decode_timeout("123H") == 123 * 3_600_000
+  end
+end