Address PR review comments

Author: rculbertson

modal-go/invocation.go

@@ -16,9 +16,6 @@ type invocation interface {
 	retry(retryCount uint32) error
 }
 
-// getOutput is a function type that takes a timeout and returns a FunctionGetOutputsItem or nil, and an error.
-type getOutput func(timeout time.Duration) (*pb.FunctionGetOutputsItem, error)
-
 // controlPlaneInvocation implements the invocation interface.
 type controlPlaneInvocation struct {
 	FunctionCallId  string
@@ -171,7 +168,13 @@ func (i *inputPlaneInvocation) retry(retryCount uint32) error {
 	return nil
 }
 
-// pollFunctionOutput repeatedly fetches the output for a given function call, waiting until a result is available or a timeout occurs.
+// getOutput is a function type that takes a timeout and returns a FunctionGetOutputsItem or nil, and an error.
+// Used by `pollForOutputs` to fetch from either the control plane or the input plane, depending on the implementation.
+type getOutput func(timeout time.Duration) (*pb.FunctionGetOutputsItem, error)
+
+// pollFunctionOutput repeatedly tries to fetch an output using the provided `getOutput` function, and the specified
+// timeout value. We use a timeout value of 55 seconds if the caller does not specify a timeout value, or if the
+// specified timeout value is greater than 55 seconds.
 func pollFunctionOutput(ctx context.Context, getOutput getOutput, timeout *time.Duration) (any, error) {
 	startTime := time.Now()
 	pollTimeout := outputsTimeout

modal-js/src/client.ts

@@ -39,19 +39,15 @@ function authMiddleware(profile: Profile): ClientMiddleware {
       if (token) {
         modalAuthToken = token;
       }
-      if (prevOnHeader) {
-        prevOnHeader(header);
-      }
+      prevOnHeader?.(header);
     };
     const prevOnTrailer = options.onTrailer;
     options.onTrailer = (trailer) => {
       const token = trailer.get("x-modal-auth-token");
       if (token) {
         modalAuthToken = token;
       }
-      if (prevOnTrailer) {
-        prevOnTrailer(trailer);
-      }
+      prevOnTrailer?.(trailer);
     };
     return yield* call.next(call.request, options);
   };

modal-js/src/invocation.ts

@@ -29,14 +29,6 @@ export interface Invocation {
   retry(retryCount: number): Promise<void>;
 }
 
-/**
- * Signature of a function that fetches a single output. Used by `pollForOutputs` to fetch from either
- * the control plane or the input plane, depending on the implementation.
- */
-type GetOutput = (
-  timeoutMillis: number,
-) => Promise<FunctionGetOutputsItem | undefined>;
-
 /**
  * Implementation of Invocation which sends inputs to the control plane.
  */
@@ -97,19 +89,15 @@ export class ControlPlaneInvocation implements Invocation {
   async #getOutput(
     timeoutMillis: number,
   ): Promise<FunctionGetOutputsItem | undefined> {
-    try {
-      const response = await client.functionGetOutputs({
-        functionCallId: this.functionCallId,
-        maxValues: 1,
-        timeout: timeoutMillis / 1000, // Backend needs seconds
-        lastEntryId: "0-0",
-        clearOnSuccess: true,
-        requestedAt: timeNowSeconds(),
-      });
-      return response.outputs ? response.outputs[0] : undefined;
-    } catch (err) {
-      throw new Error(`FunctionGetOutputs failed: ${err}`);
-    }
+    const response = await client.functionGetOutputs({
+      functionCallId: this.functionCallId,
+      maxValues: 1,
+      timeout: timeoutMillis / 1000, // Backend needs seconds
+      lastEntryId: "0-0",
+      clearOnSuccess: true,
+      requestedAt: timeNowSeconds(),
+    });
+    return response.outputs ? response.outputs[0] : undefined;
   }
 
   async retry(retryCount: number): Promise<void> {
@@ -212,6 +200,19 @@ function timeNowSeconds() {
   return Date.now() / 1e3;
 }
 
+/**
+ * Signature of a function that fetches a single output using the given timeout. Used by `pollForOutputs` to fetch
+ * from either the control plane or the input plane, depending on the implementation.
+ */
+type GetOutput = (
+  timeoutMillis: number,
+) => Promise<FunctionGetOutputsItem | undefined>;
+
+/***
+ * Repeatedly tries to fetch an output using the provided `getOutput` function, and the specified timeout value.
+ * We use a timeout value of 55 seconds if the caller does not specify a timeout value, or if the specified timeout
+ * value is greater than 55 seconds.
+ */
 async function pollFunctionOutput(
   getOutput: GetOutput,
   timeout?: number, // in milliseconds

test-support/libmodal_test_support.py

@@ -19,10 +19,12 @@ def sleep(t: int) -> None:
 def bytelength(buf: bytes) -> int:
     return len(buf)
 
+
 @app.function(min_containers=1, experimental_options={"input_plane_region": "us-west"})
 def input_plane(s: str) -> str:
     return "output: " + s
 
+
 @app.cls(min_containers=1)
 class EchoCls:
     @modal.method()