diff --git a/grpc/test/grpc/adapters/mint_test.exs b/grpc/test/grpc/adapters/mint_test.exs index e5e23e633..ede5b958b 100644 --- a/grpc/test/grpc/adapters/mint_test.exs +++ b/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) diff --git a/grpc/test/grpc/transport/utils_test.exs b/grpc/test/grpc/transport/utils_test.exs deleted file mode 100644 index 244dcbde7..000000000 --- a/grpc/test/grpc/transport/utils_test.exs +++ /dev/null @@ -1,78 +0,0 @@ -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 1000 - @second_ceiling @ms_ceiling * 60 - @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(@ms_ceiling - 1) == "999m" - end - - test "encode_timeout/1 returns second" do - assert encode_timeout(@ms_ceiling) == "1S" - assert encode_timeout(@second_ceiling - 1) == "59S" - end - - test "encode_timeout/1 returns minute" do - assert encode_timeout(@second_ceiling) == "1M" - assert encode_timeout(@minute_ceiling - 1) == "59M" - end - - test "encode_timeout/1 returns hour" do - assert encode_timeout(@minute_ceiling) == "1H" - assert encode_timeout(@minute_ceiling * 24) == "24H" - 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 diff --git a/grpc_core/lib/grpc/time_utils.ex b/grpc_core/lib/grpc/time_utils.ex index 4acdcb8f2..dc2b9b80e 100644 --- a/grpc_core/lib/grpc/time_utils.ex +++ b/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 diff --git a/grpc_core/lib/grpc/transport/utils.ex b/grpc_core/lib/grpc/transport/utils.ex index 5e2b33181..5e6fe3567 100644 --- a/grpc_core/lib/grpc/transport/utils.ex +++ b/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 """ diff --git a/grpc_core/test/grpc/time_utils_test.exs b/grpc_core/test/grpc/time_utils_test.exs index 588130ccd..7a0bdf6cb 100644 --- a/grpc_core/test/grpc/time_utils_test.exs +++ b/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 diff --git a/grpc_core/test/grpc/transport/http2_timeout_test.exs b/grpc_core/test/grpc/transport/http2_timeout_test.exs new file mode 100644 index 000000000..79ce6837c --- /dev/null +++ b/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 diff --git a/grpc_core/test/grpc/transport/utils_test.exs b/grpc_core/test/grpc/transport/utils_test.exs new file mode 100644 index 000000000..c140c9221 --- /dev/null +++ b/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