Eagerly throw on null request parameters (#1920)

Eagerly throw on null request parameters By throwing when the null values are inserted into the Request builder it's possible to get a helpful stack trace instead of one that just traces back into one of Dialogue's async threads.
palantir · Apr 28, 2023 · 9c78ef0 · 9c78ef0
1 parent 7bb7954
commit 9c78ef0
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 0 deletions.
diff --git a/changelog/@unreleased/pr-1920.v2.yml b/changelog/@unreleased/pr-1920.v2.yml
@@ -0,0 +1,8 @@
+type: improvement
+improvement:
+  description: Eagerly throw on null request parameters (header names, query parameter
+    names and values, path parameter names and values) to provide helpful stack traces
+    into user code when null values are inserted into the Request builder instead
+    of stack traces that just point to Dialogue's async threads.
+  links:
+  - https://github.com/palantir/dialogue/pull/1920
diff --git a/dialogue-target/src/main/java/com/palantir/dialogue/Request.java b/dialogue-target/src/main/java/com/palantir/dialogue/Request.java
@@ -187,6 +187,7 @@ public Request.Builder putHeaderParams(String key, String... values) {
         }
 
         public Request.Builder putHeaderParams(String key, String value) {
+            Preconditions.checkNotNull(key, "Header name must not be null");
             mutableHeaderParams().put(key, value);
             return this;
         }
@@ -197,6 +198,7 @@ public Request.Builder headerParams(Multimap<String, ? extends String> entries)
         }
 
         public Request.Builder putAllHeaderParams(String key, Iterable<String> values) {
+            Preconditions.checkNotNull(key, "Header name must not be null");
             mutableHeaderParams().putAll(key, values);
             return this;
         }
@@ -207,16 +209,21 @@ public Request.Builder putAllHeaderParams(Multimap<String, ? extends String> ent
         }
 
         public Request.Builder putQueryParams(String key, String... values) {
+            Preconditions.checkNotNull(key, "Query parameter name must not be null");
             mutableQueryParams().putAll(key, Arrays.asList(values));
             return this;
         }
 
         public Request.Builder putQueryParams(String key, String value) {
+            Preconditions.checkNotNull(key, "Query parameter name must not be null");
+            Preconditions.checkNotNull(value, "Query parameter value must not be null");
             mutableQueryParams().put(key, value);
             return this;
         }
 
         public Request.Builder putQueryParams(Map.Entry<String, ? extends String> entry) {
+            Preconditions.checkNotNull(entry.getKey(), "Query parameter name must not be null");
+            Preconditions.checkNotNull(entry.getValue(), "Query parameter value must not be null");
             mutableQueryParams().put(entry.getKey(), entry.getValue());
             return this;
         }
@@ -227,6 +234,7 @@ public Request.Builder queryParams(Multimap<String, ? extends String> entries) {
         }
 
         public Request.Builder putAllQueryParams(String key, Iterable<String> values) {
+            Preconditions.checkNotNull(key, "Query parameter name must not be null");
             mutableQueryParams().putAll(key, values);
             return this;
         }
@@ -237,11 +245,15 @@ public Request.Builder putAllQueryParams(Multimap<String, ? extends String> entr
         }
 
         public Request.Builder putPathParams(String key, String value) {
+            Preconditions.checkNotNull(key, "Path parameter name must not be null");
+            Preconditions.checkNotNull(value, "Path parameter value must not be null");
             mutablePathParams().put(key, value);
             return this;
         }
 
         public Request.Builder putPathParams(Map.Entry<String, ? extends String> entry) {
+            Preconditions.checkNotNull(entry.getKey(), "Path parameter name must not be null");
+            Preconditions.checkNotNull(entry.getValue(), "Path parameter value must not be null");
             mutablePathParams().put(entry.getKey(), entry.getValue());
             return this;
         }
@@ -252,6 +264,7 @@ public Request.Builder pathParams(Map<String, ? extends String> entries) {
         }
 
         public Request.Builder putAllPathParams(String key, Iterable<String> values) {
+            Preconditions.checkArgumentNotNull(key, "Path parameter name must not be null");
             mutablePathParams().putAll(key, values);
             return this;
         }