Handle null contents gracefully in message passing API (apple#657)

In messages "Read Resource Response" and "Read Module Response", if `contents` and `error` are both null, default to an empty byte array/string. This resolves one of the issues reported in apple#656
holzensp · Oct 23, 2024 · be95ea2 · be95ea2
1 parent a878bf8
commit be95ea2
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 2 deletions.
diff --git a/docs/modules/bindings-specification/pages/message-passing-api.adoc b/docs/modules/bindings-specification/pages/message-passing-api.adoc
@@ -414,6 +414,8 @@ The response to <<read-resource-request>>.
 If successful, `contents` is set.
 Otherwise, `error` is set.
 
+If neither is set, `contents` defaults to an empty byte array.
+
 [source,pkl]
 ----
 /// A number identifying this request.
@@ -463,6 +465,8 @@ The response to <<read-module-request>>.
 If successful, `contents` is set.
 Otherwise, `error` is set.
 
+If neither is set, `contents` defaults to an empty string.
+
 [source,pkl]
 ----
 /// A number identifying this request.

diff --git a/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt b/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt
@@ -74,7 +74,7 @@ internal class ClientModuleKeyFactory(
                     if (response.error != null) {
                       completeExceptionally(IOException(response.error))
                     } else {
-                      complete(response.contents!!)
+                      complete(response.contents ?: "")
                     }
                   }
                   else -> {

diff --git a/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt b/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt
@@ -91,7 +91,7 @@ internal class ClientResourceReader(
                 if (response.error != null) {
                   completeExceptionally(IOException(response.error))
                 } else {
-                  complete(response.contents!!)
+                  complete(response.contents ?: ByteArray(0))
                 }
               }
               else -> {

diff --git a/pkl-server/src/test/kotlin/org/pkl/server/AbstractServerTest.kt b/pkl-server/src/test/kotlin/org/pkl/server/AbstractServerTest.kt
@@ -180,6 +180,43 @@ abstract class AbstractServerTest {
     assertThat(value.asStringValue().asString()).isEqualTo("my bahumbug")
   }
 
+  @Test
+  fun `read resource -- null contents and null error`() {
+    val reader =
+      ResourceReaderSpec(scheme = "bahumbug", hasHierarchicalUris = true, isGlobbable = false)
+    val evaluatorId = client.sendCreateEvaluatorRequest(resourceReaders = listOf(reader))
+
+    client.send(
+      EvaluateRequest(
+        requestId = 1,
+        evaluatorId = evaluatorId,
+        moduleUri = URI("repl:text"),
+        moduleText = """res = read("bahumbug:/foo.pkl").text""",
+        expr = "res"
+      )
+    )
+
+    val readResourceMsg = client.receive<ReadResourceRequest>()
+    assertThat(readResourceMsg.uri.toString()).isEqualTo("bahumbug:/foo.pkl")
+    assertThat(readResourceMsg.evaluatorId).isEqualTo(evaluatorId)
+
+    client.send(
+      ReadResourceResponse(
+        requestId = readResourceMsg.requestId,
+        evaluatorId = evaluatorId,
+        contents = null,
+        error = null
+      )
+    )
+
+    val evaluateResponse = client.receive<EvaluateResponse>()
+    assertThat(evaluateResponse.error).isNull()
+
+    val unpacker = MessagePack.newDefaultUnpacker(evaluateResponse.result)
+    val value = unpacker.unpackValue()
+    assertThat(value.asStringValue().asString()).isEqualTo("")
+  }
+
   @Test
   fun `read resource error`() {
     val reader =
@@ -391,6 +428,49 @@ abstract class AbstractServerTest {
     assertThat(value.asIntegerValue().asInt()).isEqualTo(5)
   }
 
+  @Test
+  fun `read module -- null contents and null error`() {
+    val reader =
+      ModuleReaderSpec(
+        scheme = "bird",
+        hasHierarchicalUris = true,
+        isLocal = true,
+        isGlobbable = false
+      )
+    val evaluatorId = client.sendCreateEvaluatorRequest(moduleReaders = listOf(reader))
+
+    client.send(
+      EvaluateRequest(
+        requestId = 1,
+        evaluatorId = evaluatorId,
+        moduleUri = URI("repl:text"),
+        moduleText = """res = import("bird:/pigeon.pkl")""",
+        expr = "res"
+      )
+    )
+
+    val readModuleMsg = client.receive<ReadModuleRequest>()
+    assertThat(readModuleMsg.uri.toString()).isEqualTo("bird:/pigeon.pkl")
+    assertThat(readModuleMsg.evaluatorId).isEqualTo(evaluatorId)
+
+    client.send(
+      ReadModuleResponse(
+        requestId = readModuleMsg.requestId,
+        evaluatorId = evaluatorId,
+        contents = null,
+        error = null
+      )
+    )
+
+    val evaluateResponse = client.receive<EvaluateResponse>()
+    assertThat(evaluateResponse.error).isNull()
+    val unpacker = MessagePack.newDefaultUnpacker(evaluateResponse.result)
+    val value = unpacker.unpackValue().asArrayValue().list()
+    assertThat(value[0].asIntegerValue().asLong()).isEqualTo(0x1)
+    assertThat(value[1].asStringValue().asString()).isEqualTo("pigeon")
+    assertThat(value[3].asArrayValue().list()).isEmpty()
+  }
+
   @Test
   fun `read module error`() {
     val reader =