[dotnet] Improve handling for missing status code in HTTP interception #15935
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
RenderMichael
commented
Jun 23, 2025
| @@ -192,7 +192,7 @@ public override async Task ContinueRequestWithResponse(HttpRequestData requestDa | |||
| var commandSettings = new FulfillRequestCommandSettings() | |||
| { | |||
| RequestId = requestData.RequestId, | |||
| ResponseCode = responseData.StatusCode, | |||
| ResponseCode = responseData.StatusCode ?? throw new ArgumentException("Response data status code cannot be missing", nameof(responseData)), | |||
There was a problem hiding this comment.
Best to throw loudly instead of concealing issues
|
We should investigate deeper, I have reproduced it locally and see strange behavior. I also tried the same in BiDi implementation, and see correct behavior. So when we see the following: networkInterceptor.NetworkResponseReceived += (sender, args) =>
{
Console.WriteLine($"Received HTTP response: {args.ResponseUrl} with status code: {args.ResponseStatusCode}");
};What I understand we should not see Gathered some logs: BiDi, NavigateAsync command throws: CDP, classic NavigateAsync command success: Given that |
|
Check out my discovery in the original issue: #15844 (comment) DevTools does not get back any status code. The selenium side of things should handle that scenario. |
|
Your comment is understandable: Insomnia is not a browser, it just sent request to remote end and got response (401). Browser works differently, and selenium relies on CDP. Let's look closer at CDP logs, and try to understand why we got response without status code. |
This is really key point to investigate, in my head this is absolutely incorrect. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
Related to (but does not close) #15844
Minor breaking change here, but it is warranted; sometimes the HTTP status code is not present (such as the repro in the linked issue).
💥 What does this PR do?
This PR handles situations where no HTTP status code was returned by a given HTTP call.
🔄 Types of changes
PR Type
Bug fix
Description
Handle missing HTTP status codes in DevTools network interception
Change
StatusCodeproperty fromlongtolong?for nullable supportUpdate DevTools versions v135, v136, v137 to use
GetValueOrDefault()Fix potential null reference exceptions in network response handling
Changes walkthrough 📝
V135Network.cs
Handle null status codes in v135 networkdotnet/src/webdriver/DevTools/v135/V135Network.cs
ResponseCodeassignment to useGetValueOrDefault()methodStatusCodemight be null in request fulfillmentV136Network.cs
Handle null status codes in v136 networkdotnet/src/webdriver/DevTools/v136/V136Network.cs
ResponseCodeassignment to useGetValueOrDefault()methodStatusCodemight be null in request fulfillmentV137Network.cs
Handle null status codes in v137 networkdotnet/src/webdriver/DevTools/v137/V137Network.cs
ResponseCodeassignment to useGetValueOrDefault()methodGetValueOrDefault()fromStatusCodeassignment in event handlerStatusCodemight be null in both contextsHttpResponseData.cs
Make StatusCode property nullabledotnet/src/webdriver/HttpResponseData.cs
StatusCodeproperty type fromlongtolong?NetworkResponseReceivedEventArgs.cs
Make ResponseStatusCode property nullabledotnet/src/webdriver/NetworkResponseReceivedEventArgs.cs
ResponseStatusCodeproperty type fromlongtolong?