Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

add instrumentation for intermittent DNS failures #34934

Merged
merged 4 commits into from
Jan 30, 2019
Merged

add instrumentation for intermittent DNS failures #34934

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e462195
add instrumentation for #32797
wfurt Jan 29, 2019
9644cc5
Merge branch 'master' of https://github.com/wfurt/corefx into test_ex…
wfurt Jan 29, 2019
32f26f8
actually retry the lookup
wfurt Jan 29, 2019
28ffd6b
use PlatformID.Unix
wfurt Jan 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/System.Net.NameResolution/tests/PalTests/NameResolutionPalTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public void TryGetAddrInfo_LocalHost()
Assert.NotNull(hostEntry.Aliases);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotArm64Process))] // [ActiveIssue(32797)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test case inside a test project, we need [Fact]/[Theory] label to run the test. Here seems we remove the [ConditionalFact] without adding a new [Fact] label, and I don't think the test will actually run - or this is intentional? not running the test?

cc: @davidsh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. And there is another test in this file also that now no longer runs. It needs to be changed to "[Fact]" as well.

public void TryGetAddrInfo_HostName()
{
string hostName = NameResolutionPal.GetHostName();
Expand All @@ -50,6 +49,16 @@ public void TryGetAddrInfo_HostName()
// machine configurations, which varies by distro and is often inconsistent.
return;
}

// Temporary instrumentation for #32797
if (error == SocketError.TryAgain && Environment.OSVersion.Platform == PlatformID.Unix)
{
error = NameResolutionPal.TryGetAddrInfo(hostName, out hostEntry, out nativeErrorCode);
if (error != SocketError.TryAgain)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
throw new InvalidOperationException("Name resolution failure preventable with retry");
}
}

Assert.Equal(SocketError.Success, error);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test just failed again at this assert, meaning the retry didn't help, at least not in that case:
https://mc.dot.net/#/user/dotnet-bot/pr~2Fdotnet~2Fcorefx~2Frefs~2Fpull~2F34956~2Fmerge/test~2Ffunctional~2Fcli~2F/20190130.31/workItem/System.Net.NameResolution.Pal.Tests/analysis/xunit/System.Net.NameResolution.PalTests.NameResolutionPalTests~2FTryGetAddrInfo_HostName

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another:
https://mc.dot.net/#/user/dotnet-bot/pr~2Fdotnet~2Fcorefx~2Frefs~2Fpull~2F35024~2Fmerge/test~2Ffunctional~2Fcli~2F/20190201.12/workItem/System.Net.NameResolution.Pal.Tests/analysis/xunit/System.Net.NameResolution.PalTests.NameResolutionPalTests~2FTryGetAddrInfo_HostName

Assert.NotNull(hostEntry);
Expand Down Expand Up @@ -92,7 +101,6 @@ public void TryGetAddrInfo_LocalHost_TryGetNameInfo()
Assert.NotNull(name);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotArm64Process))] // [ActiveIssue(32797)]
public void TryGetAddrInfo_HostName_TryGetNameInfo()
{
string hostName = NameResolutionPal.GetHostName();
Expand All @@ -109,6 +117,16 @@ public void TryGetAddrInfo_HostName_TryGetNameInfo()
return;
}

// Temporary instrumentation for #32797
if (error == SocketError.TryAgain && (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.FreeBSD)))
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
error = NameResolutionPal.TryGetAddrInfo(hostName, out hostEntry, out nativeErrorCode);
if (error != SocketError.TryAgain)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
throw new InvalidOperationException("Name resolution failure preventable with retry");
}
}

Assert.Equal(SocketError.Success, error);
Assert.NotNull(hostEntry);

Expand Down