You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The TryGetCommandResponseType methods return null for commandResponseType but the XML docs don't indicate this possibility. Consider documenting the null return case.
/// <summary>/// Gets the command response type corresponding to the specified command type./// </summary>/// <typeparam name="T">The type of command for which to retrieve the response type.</typeparam>/// <param name="commandResponseType">The returned response type.</param>/// <returns><see langword="true"/> if the specified command type has a mapped response type; otherwise, <see langword="false"/>.</returns>publicboolTryGetCommandResponseType<T>([NotNullWhen(true)]outType?commandResponseType)whereT:ICommand{returncommandResponseTypeDictionary.TryGetValue(typeof(T),outcommandResponseType);}/// <summary>/// Gets the command response type corresponding to the specified command type./// </summary>/// <param name="command">The type of command for which to retrieve the response type.</param>/// <param name="commandResponseType">The returned response type.</param>/// <returns><see langword="true"/> if the specified command type has a mapped response type; otherwise, <see langword="false"/>.</returns>publicboolTryGetCommandResponseType(ICommandcommand,[NotNullWhen(true)]outType?commandResponseType){returncommandResponseTypeDictionary.TryGetValue(command.GetType(),outcommandResponseType);}}
The OnFetchRequestPaused method should check if requestData is not null before using it in the constructor to prevent potential null reference exceptions.
-RequestPausedEventArgs wrapped = new RequestPausedEventArgs(null, requestData);+if (requestData != null) {+ RequestPausedEventArgs wrapped = new RequestPausedEventArgs(null, requestData);+ this.OnRequestPaused(wrapped);+}
Apply this suggestion
Suggestion importance[1-10]: 7
Why: The suggestion addresses a potential null reference exception by adding a null check before using requestData in the constructor, which is a valid defensive programming practice.
7
Add null check for responseData
The OnFetchRequestPaused method should check if responseData is not null before using it in the constructor to prevent potential null reference exceptions.
-ResponsePausedEventArgs wrappedResponse = new ResponsePausedEventArgs(responseData);+if (responseData != null) {+ ResponsePausedEventArgs wrappedResponse = new ResponsePausedEventArgs(responseData);+ if (e.ResponseStatusCode.HasValue) {+}
Apply this suggestion
Suggestion importance[1-10]: 7
Why: The suggestion adds important null validation before using responseData in the constructor, preventing potential runtime exceptions and improving code robustness.
DevTools will be deprecated, I am afraid it is risky to make any "minor" changes in this area. Except you are sure these minor changes are absolutely safe.
These changes are completely safe. The event args are never serialized or expected to be initialized by users. They just have an event associated with each one, which users can subscribe to. I avoided null checks from properties which are returned by cdp, just in case.
99% of the code changes just add an explicit constructor.
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
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
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Annotates nullability on devtools event args types, and adds explicit constructors that ensures non-nullable reference types are initialized.
Motivation and Context
Contributes to #14640
Types of changes
Checklist
PR Type
enhancement, bug fix
Description
Annotated nullability for DevTools event arguments and related classes.
Added explicit constructors to ensure proper initialization of non-nullable fields.
Improved type safety with null checks and exception handling for invalid arguments.
Refactored event handling logic for better readability and maintainability.
Changes walkthrough 📝
20 files
Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Updated method to return nullable type.Added null checks and annotations for type safety.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added null checks and annotations for constructor arguments.Added null checks and annotations for constructor arguments.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Added nullability annotations and explicit constructor.Refactored event handling for better readability.Refactored log entry handling with improved initialization.Refactored network event handling with improved initialization.Refactored target event handling with improved initialization.Refactored event handling for better readability.11 files