[dotnet] [bidi] Implement browsing context download events by nvborisenko · Pull Request #16382 · SeleniumHQ/selenium

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Throw exception on unknown discriminator

In the Read method, throw a JsonException for unknown status discriminators
instead of returning null to prevent silent failures and improve error handling.

dotnet/src/webdriver/BiDi/Communication/Json/Converters/Polymorphic/DownloadEndEventArgsConverter.cs [31-39]

 public override DownloadEndEventArgs? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
 {
-    return reader.GetDiscriminator("status") switch
+    var discriminator = reader.GetDiscriminator("status");
+    return discriminator switch
     {
         "canceled" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<DownloadCanceledEventArgs>()),
         "complete" => JsonSerializer.Deserialize(ref reader, options.GetTypeInfo<DownloadCompleteEventArgs>()),
-        _ => null,
+        _ => throw new JsonException($"Unknown 'status' discriminator '{discriminator}' for DownloadEndEventArgs."),
     };
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that returning null for an unknown discriminator can hide issues. Throwing an exception improves robustness by making failures explicit, which is a good practice for deserialization logic.

Medium
  • Update