DRAFT: change default mode for tesla adapter to passive#812
DRAFT: change default mode for tesla adapter to passive#812flaviogrossi wants to merge 2 commits intoelixir-tesla:masterfrom
Conversation
when using the tesla adapter in passive mode, the timeout transport error is
wrapped in a string like "Encounter Mint error %Mint.TransportError{reason:
:timeout}", which is not coherent with the {:error, :timeout} error returned
when in active mode.
this fixes issues elixir-tesla#357 and elixir-tesla#811
| _ -> opts | ||
| end | ||
|
|
||
| opts = Map.put_new(opts, :mode, :passive) |
There was a problem hiding this comment.
as far as i can tell put_new only add :passive when the :mode is not defined at all, effectively, acting as a fallback.
Can we prove that this isn't working?
There was a problem hiding this comment.
i was under the impression that the result of the discussion on that bug report was to switch the default options for the mint adapter to use the passive mode (see here and here). Am i wrong?
I think it makes sense, since it is not working reliably if the library user is not explicitly passing the mode option (if not, it will use active).
I can reproduce the problem locally in a consistent way, but it requires a complex setup, i doubt i will be able to provide a minimal example. In any case, the issue goes away by either switching the default mode like proposed here, or explicitly providing the mode option in the caller code (which i'm not sure if we should require the user to do so)
There was a problem hiding this comment.
try to provide the setup, and logs as much as you can, it will help me to help you back, keep adding dbg 😄
There was a problem hiding this comment.
i'm adding more details in the issue. Will try to add more as soon as i can
2 participants
this should fix #811
Reading the issue #357 and the subsequent PR, it seems that the desired fix was to set the default mint mode to passive, however it was not really set because of the reasons outlined in #811
A related change is to avoid wrapping the timeout transport error in a string when passive mode is used: currently the
receive_messagefunction returns a{:error, :timeout}error when in active mode, a mint error when in passive, which is then wrapped in a{:error, "Encounter Mint error %Mint.TransportError{reason: :timeout}"}tuple. This was changed to avoid incoherent errors between the two modes.Please let me know if this is an acceptable way to fix the timeout return value inconsistency or it is better to leave it as it is (and instead fix the test) because it would technically introduce a breaking change in the api