Skip to content

[dotnet] Address nullable warnings #15389

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft

[dotnet] Address nullable warnings #15389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions dotnet/src/webdriver/DevTools/v131/V131Network.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public override async Task ContinueRequestWithResponse(HttpRequestData requestDa

var commandSettings = new FulfillRequestCommandSettings()
{
RequestId = requestData.RequestId,
RequestId = requestData.RequestId!,
Copy link
Member

Choose a reason for hiding this comment

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

At first glance requestData.RequestId should not be nullable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RequestId can be null in one specific circumstance: when a user creates an HttpRequestData themselves.

This only happens in one place, and it can even cause problems. I filed an issue about it: #15380

However, these HttpRequestDatas are known to be created internally, so we know RequestId is not null.

Copy link
Member

Choose a reason for hiding this comment

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

Good, so requestData.RequestId should not be null. Then I propose to not suppress this in this PR, because of we will definitely forget re-suppress it. Let's try to fix root cause, even if it will be a breaking change.

What I understand, we have 2 ways:

  • Slightly adjust public API to require RequestID to be not nullable (following standard policy of deprecation), will be a part of v4
  • Entirely revisit public API of NetworkManager in v5 (how this API looks, how it could look, including renaming StartMonitoring to StartMonitoringAsync)

@RenderMichael if can manage the 1st way easily with minimal effort, it would be great (as short-term solution before v5).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds like a much better solution.

All 3 proposed solutions fix the issue (2 is my favorite but breaking, 3 is still good, 1 is the backup which is not a breaking change).

ResponseCode = responseData.StatusCode,
};

Expand Down Expand Up @@ -232,7 +232,7 @@ public override async Task ContinueRequestWithoutModification(HttpRequestData re
throw new ArgumentNullException(nameof(requestData));
}

await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId }).ConfigureAwait(false);
await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId! }).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -289,7 +289,7 @@ public override async Task AddResponseBody(HttpResponseData responseData)
// If the response is a redirect, retrieving the body will throw an error in CDP.
if (responseData.StatusCode < 300 || responseData.StatusCode > 399)
{
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
if (bodyResponse != null)
{
if (bodyResponse.Base64Encoded)
Expand Down Expand Up @@ -317,7 +317,7 @@ public override async Task ContinueResponseWithoutModification(HttpResponseData
throw new ArgumentNullException(nameof(responseData));
}

await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
}

private void OnFetchAuthRequired(object? sender, Fetch.AuthRequiredEventArgs e)
Expand Down
8 changes: 4 additions & 4 deletions dotnet/src/webdriver/DevTools/v132/V132Network.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public override async Task ContinueRequestWithResponse(HttpRequestData requestDa

var commandSettings = new FulfillRequestCommandSettings()
{
RequestId = requestData.RequestId,
RequestId = requestData.RequestId!,
ResponseCode = responseData.StatusCode,
};

Expand Down Expand Up @@ -232,7 +232,7 @@ public override async Task ContinueRequestWithoutModification(HttpRequestData re
throw new ArgumentNullException(nameof(requestData));
}

await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId }).ConfigureAwait(false);
await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId! }).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -289,7 +289,7 @@ public override async Task AddResponseBody(HttpResponseData responseData)
// If the response is a redirect, retrieving the body will throw an error in CDP.
if (responseData.StatusCode < 300 || responseData.StatusCode > 399)
{
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
if (bodyResponse != null)
{
if (bodyResponse.Base64Encoded)
Expand Down Expand Up @@ -317,7 +317,7 @@ public override async Task ContinueResponseWithoutModification(HttpResponseData
throw new ArgumentNullException(nameof(responseData));
}

await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
}

private void OnFetchAuthRequired(object? sender, Fetch.AuthRequiredEventArgs e)
Expand Down
8 changes: 4 additions & 4 deletions dotnet/src/webdriver/DevTools/v133/V133Network.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public override async Task ContinueRequestWithResponse(HttpRequestData requestDa

var commandSettings = new FulfillRequestCommandSettings()
{
RequestId = requestData.RequestId,
RequestId = requestData.RequestId!,
ResponseCode = responseData.StatusCode,
};

Expand Down Expand Up @@ -232,7 +232,7 @@ public override async Task ContinueRequestWithoutModification(HttpRequestData re
throw new ArgumentNullException(nameof(requestData));
}

await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId }).ConfigureAwait(false);
await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId! }).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -289,7 +289,7 @@ public override async Task AddResponseBody(HttpResponseData responseData)
// If the response is a redirect, retrieving the body will throw an error in CDP.
if (responseData.StatusCode < 300 || responseData.StatusCode > 399)
{
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
if (bodyResponse != null)
{
if (bodyResponse.Base64Encoded)
Expand Down Expand Up @@ -317,7 +317,7 @@ public override async Task ContinueResponseWithoutModification(HttpResponseData
throw new ArgumentNullException(nameof(responseData));
}

await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
await fetch.ContinueResponse(new ContinueResponseCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
}

private void OnFetchAuthRequired(object? sender, Fetch.AuthRequiredEventArgs e)
Expand Down
8 changes: 4 additions & 4 deletions dotnet/src/webdriver/DevTools/v85/V85Network.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public override async Task ContinueRequestWithResponse(HttpRequestData requestDa

var commandSettings = new FulfillRequestCommandSettings()
{
RequestId = requestData.RequestId,
RequestId = requestData.RequestId!,
ResponseCode = responseData.StatusCode,
};

Expand Down Expand Up @@ -232,7 +232,7 @@ public override async Task ContinueRequestWithoutModification(HttpRequestData re
throw new ArgumentNullException(nameof(requestData));
}

await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId }).ConfigureAwait(false);
await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = requestData.RequestId! }).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -289,7 +289,7 @@ public override async Task AddResponseBody(HttpResponseData responseData)
// If the response is a redirect, retrieving the body will throw an error in CDP.
if (responseData.StatusCode < 300 || responseData.StatusCode > 399)
{
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
var bodyResponse = await fetch.GetResponseBody(new Fetch.GetResponseBodyCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
if (bodyResponse.Base64Encoded)
{
responseData.Content = new HttpResponseContent(Convert.FromBase64String(bodyResponse.Body));
Expand All @@ -314,7 +314,7 @@ public override async Task ContinueResponseWithoutModification(HttpResponseData
throw new ArgumentNullException(nameof(responseData));
}

await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = responseData.RequestId }).ConfigureAwait(false);
await fetch.ContinueRequest(new ContinueRequestCommandSettings() { RequestId = responseData.RequestId! }).ConfigureAwait(false);
}

private void OnFetchAuthRequired(object? sender, Fetch.AuthRequiredEventArgs e)
Expand Down
2 changes: 1 addition & 1 deletion dotnet/src/webdriver/WebDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class WebDriver : IWebDriver, ISearchContext, IJavaScriptExecutor, IFinds
/// </summary>
protected static readonly TimeSpan DefaultCommandTimeout = TimeSpan.FromSeconds(60);
private IFileDetector fileDetector = new DefaultFileDetector();
private NetworkManager network;
private NetworkManager? network;
private WebElementFactory elementFactory;

private readonly List<string> registeredCommands = new List<string>();
Expand Down
Loading