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

Intermittent NullReferenceException in Unit Tests #17

Open
NathanTurnbow opened this issue Sep 6, 2018 · 0 comments
Open

Intermittent NullReferenceException in Unit Tests #17

NathanTurnbow opened this issue Sep 6, 2018 · 0 comments

Comments

@NathanTurnbow
Copy link

NathanTurnbow commented Sep 6, 2018

In many of the draft unit tests we create a new testing context like this:

using (var http = new HttpTest()) {
    http.RespondWithJson(Fixtures.Directory.DefaultResponse);

Intermittently, the call to RespondWithJson will throw a NullReferenceException.

in flurl, the source code for this call is (currently) this:

public HttpTest RespondWithJson(object body, int status = 200, object headers = null, object cookies = null, bool replaceUnderscoreWithHyphen = true) {
	var content = new CapturedJsonContent(Settings.JsonSerializer.Serialize(body));
	return RespondWith(content, status, headers, cookies, replaceUnderscoreWithHyphen);
}

https://github.com/tmenier/Flurl/blob/80cf699ef8b153c0c0fd2324a426f54f39197fd0/src/Flurl.Http/Testing/HttpTest.cs#L92-L95

The only case where we can possibly throw a null reference exception is if either Settings is null or Settings.JsonSerializer is null. In this case it is Settings.JsonSerializer, but Settings actually has a public setter so if you want to deliberately cause a similar error you could call http.Settings = null, but we're not doing that in draft tests.

When Settings.JsonSerializer is called, it first attempts to find a local value for JsonSerializer, and if it fails to find a local setting it then looks for a local default, and ultimately falls through to the globally configured static defaults accessed at FlurlHttp.GlobalSettings. In all of the test cases, we fall all the way through to the static global settings searching for an instance of JsonSerializer that can be used to configure the JsonResponse for the test.

Accessing settings in FlurlHttp.GlobalSettings is not thread-safe to be used across tests because it has the following publicly exposed method:

public override void ResetDefaults() {
	base.ResetDefaults();
	Timeout = TimeSpan.FromSeconds(100); // same as HttpClient
	JsonSerializer = new NewtonsoftJsonSerializer(null);
	UrlEncodedSerializer = new DefaultUrlEncodedSerializer();
	FlurlClientFactory = new PerHostFlurlClientFactory();
	HttpClientFactory = new DefaultHttpClientFactory();
}

https://github.com/tmenier/Flurl/blob/80cf699ef8b153c0c0fd2324a426f54f39197fd0/src/Flurl.Http/Configuration/FlurlHttpSettings.cs#L212-L219

Here, base.ResetDefaults() empties the global settings before they are reset to the hard-coded defaults

public virtual void ResetDefaults() {
	_vals.Clear();
}

https://github.com/tmenier/Flurl/blob/80cf699ef8b153c0c0fd2324a426f54f39197fd0/src/Flurl.Http/Configuration/FlurlHttpSettings.cs#L131-L133

So there exists a period of time where any of the global settings could be null, including JsonSerializer, if ResetDefaults() is called.

We have a set of tests which call FlurlHttp.GlobalSettings.ResetDefaults() explicitly (see

).

When all of the unit tests are run in parallel, we have a chance for the global defaults to be reset at the same time that we are trying to configure a json response, possibly resulting in the observed NullReferenceException.

When this offending line is removed (below) I no longer can reproduce any intermittent test failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant