Skip to content
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.Dispose throws System.NullReferenceException with OTEL #1816

Open
vchirikov opened this issue Sep 17, 2024 · 2 comments
Open

SmtpClient.Dispose throws System.NullReferenceException with OTEL #1816

vchirikov opened this issue Sep 17, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@vchirikov
Copy link

Describe the bug

MailService.Dispose throws System.NullReferenceException

Platform:

  • OS: Windows
  • .NET Runtime: 8.0.8
  • Versions:
  • <PackageVersion Include="MailKit" Version="4.7.1.1"/>
  • <PackageVersion Include="MimeKit" Version="4.7.1"/>

Exception

System.NullReferenceException: Object reference not set to an instance of an object.
   at MailKit.Net.ClientMetrics.GetTags(Uri uri, Exception ex)
   at MailKit.Net.ClientMetrics.RecordClientDisconnected(Int64 startTimestamp, Uri uri, Exception ex)
   at MailKit.Net.Smtp.SmtpClient.RecordClientDisconnected(Exception ex)
   at MailKit.Net.Smtp.SmtpClient.Disconnect(String host, Int32 port, SecureSocketOptions options, Boolean requested)
   at MailKit.Net.Smtp.SmtpClient.Dispose(Boolean disposing)
   at MailKit.MailService.Dispose()
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.DisposeAsync()

Expected behavior

Should not throw on scope disposing.

Code Snippets

using MailKit.Net.Smtp;

var services = new ServiceCollection();

MailKit.Telemetry.Configure();
// do not use IMeterFactory constructor here, it's broken, use global metrics instead
services.AddScoped<SmtpClient>();

await using var globalSp = services.BuildServiceProvider();
await using (var scope = globalSp.CreateAsyncScope())
{
    var smtp = scope.ServiceProvider.GetRequiredService<SmtpClient>();
    if (!smtp.IsConnected) {
        await smtp.ConnectAsync(host, port, encOptions, default);
    }

    if (!smtp.IsAuthenticated) {
        await smtp.AuthenticateAsync(userName, password, default);
    }

    await smtp.SendAsync(mail, default);
    if (smtp.IsConnected) {
        await smtp.DisconnectAsync(quit: true, default);
    }
} // Dispose will throw NRE if you setup otel metrics

Additional context

If you want to reuse same SmtpClient across the DI scope it's expected that you place SmtpClient in disconnected state, but it will throw on scope Dispose because DisconnectAsync drops uri field, which is used in ClientMetrics.GetTags on disposing.

p.s. also ctor with IMeterFactory doesn't work with NRE too.

@jstedfast jstedfast added the bug Something isn't working label Sep 18, 2024
@jstedfast
Copy link
Owner

p.s. also ctor with IMeterFactory doesn't work with NRE too.

Are you calling new SmtpClient(..., meterFactory) after calling Telemetry.SmtpClient.Configure(meterFactory)? If so, it's probably because a meter has already been created with that name.

jstedfast added a commit that referenced this issue Sep 18, 2024
Also discovered some issues in CreateMetrics() methods in Telemetry class.

Fixes issue #1816
@vchirikov
Copy link
Author

vchirikov commented Sep 18, 2024

Are you calling new SmtpClient(..., meterFactory) after calling Telemetry.SmtpClient.Configure(meterFactory)?

No, firstly I tried just:

services.AddScoped(sp => new SmtpClient(new NullProtocolLogger(), sp.GetRequiredService<IMeterFactory>()));

And it throws on activation:

System.NullReferenceException: Object reference not set to an instance of an object.
   at MailKit.Net.ClientMetrics..ctor(Meter meter, String meterName, String an, String protocol)
   at MailKit.Telemetry.SmtpClient.CreateMetrics(Meter meter)
   at MailKit.Net.Smtp.SmtpClient..ctor(IProtocolLogger protocolLogger, IMeterFactory meterFactory)

because meter is null inside public ClientMetrics (Meter meter, string meterName, string an, string protocol), but looks like you already found the issue with ClientMetrics ctor + property instead of the method arg. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants