How to distinguish HTTP/1 from HTTP/2 conns without Dialyzer errors?

[gamache]

Hi folks! I have just added proper handling of stream_request_body to Mojito (see PR here), which requires me to distinguish between %Mint.HTTP1{} and %Mint.HTTP2{} in my code in order to handle window size updates.

This is of course trivial to do in code, however since both Mint.HTTP1.t() and Mint.HTTP2.t() types are opaque, I am not sure how to tell the two apart without introducing errors into my Dialyzer output:

lib/mojito/conn.ex:88: The attempt to match a term of type 
          'Elixir.Mint.HTTP':t() against the pattern 
          #{'__struct__' := 'Elixir.Mint.HTTP1'} breaks the opacity of the term
lib/mojito/conn.ex:102: The attempt to match a term of type 
          'Elixir.Mint.HTTP':t() against the pattern 
          #{'__struct__' := 'Elixir.Mint.HTTP2'} breaks the opacity of the term

Any attempt to peek into the struct causes Dialyzer output. Is there a clean way I can tell the two types apart at runtime? Thanks for any help you can provide!

[gamache]

Upon further investigation, just knowing the conn type isn't good enough -- I tested this by making Mint.conn_module/1 a public function and using it in my code. When I call Mint.HTTP2.stream_request_body Mint.HTTP2.get_window_size, I am passing a Mint.HTTP.t() where a Mint.HTTP2.t() is expected, and Dialyzer takes offense at this as well.

[ericmj]

The problem is that Dialyzer doesn't let us expose only the :__struct__ field without exposing all other fields. We can add a version function on both modules that returns :http1 | http2.

What do you think @whatyouhide?

[gamache]

@ericmj Sorry for the error in my last comment, now corrected. For this reason, a version function alone is not sufficient to pass Dialyzer checks. With the current type opacity, the cheap solution would seem to be a get_window_size function that accepted Mint.HTTP.t() and returned some sentinel value for the HTTP/1 case, but that is kind of crappy from an HTTP semantics perspective. I'm not sure there is a better solution without adding stream handling inside stream_request_body within Mint so none of it was exposed to the user (and that may be a dead end too, I haven't thought about it hard).

[ericmj]

The type is defined as Mint.HTTP.t() :: Mint.HTTP1.t() | Mint.HTTP2.t() and since Dialyzer should not reject possibly valid code I don't understand why it rejects it since it is clear it could be a HTTP2 struct.

Can you check what happens if you make Mint.HTTP.t() a non-opaque type?

[gamache]

Making the HTTP.t(), HTTP1.t(), and HTTP2.t() types non-opaque may be sufficient; I have a few other dialyzer errors I'd thought were connected to the earlier error, but may be separate. I am looking into these (though Dialyzer is not helping much).

Making only HTTP.t() non-opaque does not pass Dialyzer.

[ericmj]

Making only HTTP.t() non-opaque does not pass Dialyzer.

Do you get a different error when making this change?

[ericmj]

I don't like making the types non-opaque because the fields should not be accessed other than __struct__. This is a limitation in Dialyzer that I don't know how to work around. If you have any ideas let us know and we can reopen the issue.

[andyleclair]

@ericmj what if there was a public Mint function we could call to differentiate between HTTP1 and HTTP2?

I was thinking something like a Mint.connection_type that could take a conn and return either :http1 or :http2, that way we won't break the opacity of the struct type but consumers can still get that info

[ericmj]

That sounds like the best solution. Can you send a PR that adds Mint.HTTP.module/1 that returns either Mint.HTTP1 or Mint.HTTP2?

[andyleclair]

I'd be happy to