Skip to content

Commit

Permalink
Fix a bug that happens when sentinels report peers with IP addresses (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
whatyouhide committed Sep 9, 2024
1 parent 21f9d82 commit 11bc72d
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 18 deletions.
21 changes: 10 additions & 11 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,42 @@ on:
jobs:
test:
name: Test (Elixir ${{matrix.elixir}} | Erlang/OTP ${{matrix.otp}})
runs-on: ubuntu-20.04
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- otp: "27.0"
elixir: "1.17.0"
version-type: strict
elixir: "1.17"
os: ubuntu-latest
dialyzer: true
lint: true

- otp: "25.3"
elixir: "1.14.3"
os: ubuntu-20.04
coverage: true
version-type: loose

- otp: "23.0"
elixir: "1.12"
version-type: loose
os: ubuntu-20.04

env:
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MIX_ENV: test

steps:
- name: Clone the repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Start Docker
run: docker-compose up --detach
run: docker compose up --detach

- name: Install OTP and Elixir
uses: erlef/setup-beam@v1
with:
otp-version: ${{matrix.otp}}
elixir-version: ${{matrix.elixir}}
version-type: ${{matrix.version-type}}
otp-version: ${{ matrix.otp }}
elixir-version: ${{ matrix.elixir }}

- name: Cache dependencies
id: cache-deps
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ Redix is low-level, but it's still built to handle most things thrown at it. For

## Contributing

To run the Redix test suite you will have to have Redis running locally. Redix requires a somewhat complex setup for running tests (because it needs a few instances running, for pub/sub and sentinel). For this reason, in this repository you'll find a `docker-compose.yml` file so that you can use [Docker][docker] and [docker-compose][] to spin up all the necessary Redis instances with just one command. Make sure you have Docker installed and then just run:
To run the Redix test suite you will have to have Redis running locally. Redix requires a somewhat complex setup for running tests (because it needs a few instances running, for pub/sub and sentinel). For this reason, in this repository you'll find a `docker-compose.yml` file so that you can use [Docker][docker] and [docker compose][docker-compose] to spin up all the necessary Redis instances with just one command. Make sure you have Docker installed and then just run:

```bash
docker-compose up
docker compose up
```

Now, you're ready to run tests with the `$ mix test` command.
Expand Down
20 changes: 17 additions & 3 deletions lib/redix/connector.ex
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,16 @@ defmodule Redix.Connector do
Logger.debug(fn ->
"Sentinel reported #{sentinel_opts[:role]}: #{server_host}:#{server_port}"
end),
{:ok, server_socket, _address} <-
server_host = string_address_to_erlang(server_host),
{:ok, server_socket, address} <-
connect_directly(
String.to_charlist(server_host),
server_host,
String.to_integer(server_port),
opts
),
:ok <- verify_server_role(server_socket, opts, sentinel_opts) do
:ok = transport.close(sent_socket)
{:ok, server_socket, "#{server_host}:#{server_port}"}
{:ok, server_socket, address}
else
{cause, reason} when cause in [:error, :stop] ->
:telemetry.execute([:redix, :failed_connection], %{}, %{
Expand All @@ -171,6 +172,19 @@ defmodule Redix.Connector do
end
end

defp string_address_to_erlang(address) when is_binary(address) do
address = String.to_charlist(address)

case :inet.parse_address(address) do
{:ok, ip} -> ip
{:error, :einval} -> address
end
end

defp string_address_to_erlang(address) do
address
end

defp connect_to_sentinel(sentinel, sentinel_opts, transport) do
host = Keyword.fetch!(sentinel, :host)
port = Keyword.fetch!(sentinel, :port)
Expand Down
12 changes: 11 additions & 1 deletion lib/redix/format.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Redix.Format do
# Used for formatting things to print or log or anything like that.

@spec format_host_and_port(host, :inet.port_number()) :: String.t()
when host: {:local, String.t()} | binary()
when host: {:local, String.t()} | charlist() | binary() | :inet.ip_address()
def format_host_and_port(host, port)

def format_host_and_port({:local, path}, 0) when is_binary(path), do: path
Expand All @@ -14,4 +14,14 @@ defmodule Redix.Format do

def format_host_and_port(host, port) when is_list(host),
do: format_host_and_port(IO.chardata_to_string(host), port)

def format_host_and_port(host, port) when is_tuple(host) do
case :inet.ntoa(host) do
{:error, :einval} ->
raise ArgumentError, "invalid host: #{inspect(host)}"

host ->
format_host_and_port(host, port)
end
end
end
2 changes: 1 addition & 1 deletion test/redix/pubsub_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Redix.PubSubTest do

@moduletag :pubsub

# See docker-compose.
# See docker-compose.yml.
@port 6380

setup do
Expand Down
1 change: 1 addition & 0 deletions test/redix/sentinel_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ defmodule Redix.SentinelTest do
assert_receive {:redix_pubsub, ^pubsub, ^ref, :message, %{channel: "foo", payload: "hello"}}
end

@tag :capture_log
test "when no sentinels are reachable" do
Process.flag(:trap_exit, true)

Expand Down

0 comments on commit 11bc72d

Please sign in to comment.