-
-
Notifications
You must be signed in to change notification settings - Fork 827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SmtpClient Exception Behavior Change in 4.4.0 #1748
Comments
Part of this behavior change (useful to know server disconnected) also includes this exception for successful delivery in some cases (no way to know what the status code was, but guessing it was 2xx based on the response, so we check for that now too): |
Hmmmm...
This sounds like a bug. If a command is failing, it should throw That said, for exceptions like
I'd really like to know more about this because this sounds bad if those used to be SmtpCommandExceptions. |
The way we caught this is when switching from 4.3.0 -> 4.5.0, our outbound queue started to build up significantly. After some digging into our logs, I was able to confirm this exception change behavior since the root cause of the queue build up was that we didn't know the status code of the new volume of My working theory was that the changes/fixes made in 4.4.0 caused better detection of smtp server disconnects and thus caused the switchover of many While you're looking at this: |
Okay, so there are essentially 5 locations where an SmtpProtocolException gets thrown: 3 of them are in the response parsing logic (but none of these say "unexpectedly disconnected") and the last 2 are in the ReadAhead() and ReadAheadAsync() logic which is where the "unexpectedly disconnected" error is used. That probably means that the server really did unexpectedly disconnect and, as you have suggested, it is probably a sign that MailKit's SmtpClient stopped swallowing these like it had been previously (and I think this might be evidenced by the code changes between 4.3.0 and 4.4.0 as well). These are the changes that likely played a part in this behavior change: commit 0a5029d
commit 4310535
commit 3d1487d
commit d5770f9
|
For |
Yes, that's how we handle |
I might have to add an SmtpServiceNotAuthenticatedException to add those properties because they won't map as easily for POP3 and IMAP. |
Question: When you get |
It happens on
|
That ... doesn't make sense unless there's a bug in the SendAsync logic. I don't understand how that could happen :( That response text suggests that this is happening after the SMTP server sends I'm so lost as to what could be happening there. |
We're assuming its a 250 status code. Right now we don't know the actual status code. Since SendAsync also calls MailFrom, RcptTo, etc., is it possible it's from one of those? I guess right now, those exceptions also don't let us know which smtp action caused the exception. Maybe a property should indicate that as well on the exceptions. Given we're getting that exception mostly on 4xx/5xx, I thought what was happening is that it was a 250 response and the message accepted, but the exception was raised because SmtpClient detected the server did disconnect after accepting. |
No, it would only come as a response to sending the message content. That Do you have a |
Yes, I did have a |
Actually, my analysis is wrong I think, because it only calls I think I might need a log so I can better understand what is happening. |
Most of the Are you saying that its not expected for SmtpClient to throw |
Also, we do get a "tracking id" for every server response from gsmtp regardless the command. It's the same id that's appended to all responses for that session/transaction (at least for gsmtp). |
We have commonly seen gsmtp respond with Edit: Confirmed, this is likely in response to |
That's what I would expect and gives me some measure of relief :-)
That's a relief that it's a small number, but I'm still baffled by it because the 2.x.x would insinuate success.
Let me try to explain my thinking more clearly. What I'm saying is that the SmtpClient, after receiving a The "unexpectedly disconnected" error ONLY happens if/when the client attempts to read a response and gets an EOF indicating the remote server has closed the connection. This cannot happen as part of reading a full response because SMTP responses are structured such that the client can figure out if it needs to read more data or not without having to "try-and-fail". E.g. responses are line-based and the status code is followed by All that said, it looks like my assumptions about which command was getting this response was incorrect. It looks like it was a
Yea, and I think I jumped to the wrong conclusion earlier. I think what that "tracking code" really is is a Session ID or Correlation ID that Google likely uses for looking up session telemetry data as opposed to a "message tracking id".
Yep.
This makes WAY more sense. According to https://www.rfc-editor.org/rfc/rfc3463#section-3.2 ... the If the response was 100% ok, it should be If this is in response to a It also might be why you are getting disconnected. If the address has an issue, GMail might be deciding to drop the connection and maybe that issue is an authentication issue or maybe it's a temporary throttling issue.
Right. |
That's what I had thought as well and was surprised with the new It's does seem to be throwing in response to DATA/BDAT in many cases since our logs contain plenty of Maybe it is a bug somewhere in that case because I'm assuming you wouldn't generally detect the disconnect until the read of the next command after a DATA/BDAT response has already been processed? Here is the stack trace for one of those: |
Correct, that is the way things are expected to work which means there may be a bug. In your sample stacktrace, it looks like something is causing one of the "SendAsync" commands to fail ( One reason you may be getting more |
Most of the |
…ptions. Partial fix for issue #1748
Okay, that should improve the situation a bit. Hopefully. |
Just checked out v4.3.0 and indeed, the old 4.3.0 code used to swallow RSET exceptions. |
So, it seems in most error cases MailKit will know that the server disconnected due to the attempt to RSET after a bad DATA/BDAT 4xx/5xx response or bad MAIL FROM/RCPT TO response. In those cases, could we expect SmtpClient.IsConnected (or some other way) to report accurately (since MailKit already knows) so we can still detect/log that the server has disconnected while processing the response for our connection management logic? The |
I would never depend fully on this being accurate if IsConnected=True, but if IsConnected=False, then yes. Keep in mind that if the DATA/BDAT command is successful and then the server disconnects, SmtpClielt.IsConnected will still be True until you attempt to send the next message or call NoOpAsync(). |
I released MailKit 4.6.0 with the fixes I have for this so far which restores behavior back to what it was pre-4.4.0 |
It looks like SmtpClient had a change in 4.4.0 with its exception behavior on SendAsync. We started to get significantly more SmtpProtocolExceptions with the "unexpectedly disconnected" error message. Prior to 4.4.0, we rarely got SmtpProtocolException and they were mostly all SmtpCommandException.
It's definitely useful to know that the server has disconnected, which wasn't available before so we can better decide on reuse of the SmtpClient. However, we've now lost access to the StatusCode property that was on SmtpCommandException since many of those exceptions are now being reported as SmtpProtocolException in 4.4.0+. 4.5.0 added the server response message to SmtpProtocolException, which is definitely helpful, but still no StatusCode as before, which is incredibly important to know so we can decide what to do with the message based on the server's response.
Can a nullable StatusCode property be added to SmtpProtocolException similar to SmtpCommandException? It'd have to be nullable since it's not present in every case of SmtpProtocolException since the server could disconnect without a status code or a response.
Can a ReplyText or similar property be added to both SmtpCommandException and SmtpProtocolException (nullable ReplyText) so that we can access the raw response from the server. Right now we have to resort to parsing the exception message and taking out the "unexpectedly disconnected" (since 4.4.0+) and/or other extra added wording. It creates a task to follow/update future prefixed wording changes, etc. It'd be much better to have a property to access that. We need to reliably parse the enhanced status code from the response in all cases (success/error) and check the text to make decisions, and for troubleshooting. We've been able to easily get the success case since SendAsync returns that as of It would be nice if SmtpClient.Send/SendAsync returned a string containing the free-form text response from the server #1161.
The text was updated successfully, but these errors were encountered: