Fix errors caught by Dialyzer

[Stratus3D]

It doesn't look like Dialyzer is getting run for this project.

Below are the fixes for the errors reported by Dialyzer:

lib/broadway_kafka/brod_client.ex:83:guard_fail
The guard clause:

when _ :: Keyword.t() === nil

can never succeed.

________________________________________________________________________________
lib/broadway_kafka/brod_client.ex:114:guard_fail
The guard clause:

when _ :: pid() === nil

can never succeed.

________________________________________________________________________________
lib/broadway_kafka/kafka_client.ex:54:unknown_type
Unknown type: :brod.group_coordinator/0.
________________________________________________________________________________
done (warnings were emitted)
Halting VM with exit status 2

[Stratus3D]

fetch_config is always going to be a list that passed validation in the validate_fetch_config/1 function, so the || [] will never be used here.

[Stratus3D]

According to Dialyzer group_coordinator will always be set. It's a pid so even if the pid dies it won't be a falsey value.

[josevalim]

I believe it is getting this information from the callback but the callback is wrong. According to producer.ex the group_coordinator can definitely be nil.

[Stratus3D]

Both :brod_group_coordinator.ack/5 and :brod_group_coordinator.commit_offsets/2 both expect group_coordinator to be a pid, not nil.

I think Dialyzer is complaining about this nil check because :brod_group_coordinator.ack/5 is invoked before it, so Dialyzer assumes that if we get to this line it MUST NOT be nil (and maybe it won't ever be, if :brod_group_coordinator.ack/5 always crashes when nil is passed).

We can solve this by having both calls inside a if group_coordinator do block. For consistency we should also do the same thing for the update_topics/2 function in this module.

It feels a little strange return ok when group_coordinator coordinator is nil however, but this is the only return type allowed by the behaviour:

  @impl true
  def ack(group_coordinator, generation_id, topic, partition, offset, config) do
    if group_coordinator do
      :brod_group_coordinator.ack(group_coordinator, generation_id, topic, partition, offset)

      if config.offset_commit_on_ack do
        :brod_group_coordinator.commit_offsets(group_coordinator, [{{topic, partition}, offset}])
      end
    end

    :ok
  end

[josevalim]

Yes, I think that would be better. Although, if this is the case, it seems group_coordinator cannot be nil in this particular branch indeed.

[Stratus3D]

PR updated. I have made the changes I described above.

[Stratus3D]

I didn't see this type defined in brod so I defined one in this module.

[Stratus3D]

Refactor for consistency.

[josevalim]

💚 💙 💜 💛 ❤️

[Stratus3D]

Thanks @josevalim !