async-email / async-smtp

Apache License 2.0
59 stars 12 forks source link

Flexible timeouts (for larger messages) #22

Closed Hocuri closed 4 years ago

Hocuri commented 4 years ago

For https://github.com/deltachat/deltachat-core-rust/issues/1383 we need an different timeouts for larger messages. The options I see are:

cc @link2xt @hpk42

dignifiedquire commented 4 years ago

why does this need to live in this library and not be part of the way it is being used? On 8. Jun 2020, 09:45 +0200, Hocuri notifications@github.com, wrote:

For deltachat/deltachat-core-rust#1383 we need an different timeouts for larger messages. The options I see are:

• function to reconfigure the timeout in runtime (@link2xt didn't like this option) • a functional-style setting .timeout_extra_time_per_mb( ...3 minutes... )

cc @link2xt @hpk42 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Hocuri commented 4 years ago

How would you fix https://github.com/deltachat/deltachat-core-rust/issues/1383 without modifying this library?

link2xt commented 4 years ago

@dignifiedquire A timeout should measure the time between last byte sent and OK received. It is impossible to achieve this by adding a timeout for the whole send operation. Waiting for server response happens inside the library.

Hocuri commented 4 years ago

So @link2xt @dignifiedquire which of the options would you say is better? Or something else?

dignifiedquire commented 4 years ago

I don't think mapping timeouts to message size belongs in this library. So what I would prefer is something like

link2xt commented 4 years ago

send_with_timeout looks good to me, not storing the timeout inside the library is nice.

link2xt commented 4 years ago

Here is the list of timeouts: https://tools.ietf.org/html/rfc5321#section-4.5.3.2

We are interested in DATA Termination timeout here.