compiler/internal/codegen: Handle nil pointer responses (encoredev#254)

This PR adds logic to handle nil pointers when serializing the response in the generated request handlers.
eandre · May 30, 2022 · cfe1a46 · cfe1a46
1 parent d9de2d8
commit cfe1a46
Show file tree

Hide file tree

Showing 8 changed files with 478 additions and 89 deletions.
diff --git a/cli/daemon/run/run_test.go b/cli/daemon/run/run_test.go
@@ -271,6 +271,14 @@ func TestEndToEndWithApp(t *testing.T) {
 			c.Assert(w2.Body.Bytes(), qt.DeepEquals, w2.Body.Bytes())
 		}
 
+		// Call an endpoint without request parameters, returning nil
+		{
+			w := httptest.NewRecorder()
+			req := httptest.NewRequest("GET", "/echo.NilResponse", nil)
+			run.ServeHTTP(w, req)
+			c.Assert(w.Code, qt.Equals, 200)
+		}
+
 		// Call an endpoint without request parameters and response value
 		{
 			w := httptest.NewRecorder()

diff --git a/cli/daemon/run/testdata/echo/echo/echo.go b/cli/daemon/run/testdata/echo/echo/echo.go
@@ -116,6 +116,12 @@ func Noop(ctx context.Context) error {
 	return nil
 }
 
+// NilResponse returns a nil response and nil error
+//encore:api public method=GET,POST
+func NilResponse(ctx context.Context) (*BasicData, error) {
+	return nil, nil
+}
+
 // MuteEcho absorbs a request
 //encore:api public method=GET
 func MuteEcho(ctx context.Context, params Data[string, string]) error {

diff --git a/cli/daemon/run/testdata/echo_client/client.ts b/cli/daemon/run/testdata/echo_client/client.ts
@@ -234,6 +234,15 @@ export namespace echo {
             await this.baseClient.callAPI("GET", `/echo.MuteEcho`, undefined, {query})
         }
 
+        /**
+         * NilResponse returns a nil response and nil error
+         */
+        public async NilResponse(): Promise<BasicData> {
+            // Now make the actual call to the API
+            const resp = await this.baseClient.callAPI("POST", `/echo.NilResponse`)
+            return await resp.json() as BasicData
+        }
+
         /**
          * NonBasicEcho echoes back the request data.
          */

diff --git a/cli/daemon/run/testdata/echo_client/client/client.go b/cli/daemon/run/testdata/echo_client/client/client.go
diff --git a/compiler/internal/codegen/codegen_main.go b/compiler/internal/codegen/codegen_main.go
@@ -289,51 +289,68 @@ func (b *Builder) buildRPC(svc *est.Service, rpc *est.RPC) *Statement {
 
 func (b *Builder) encodeResponse(g *Group, rpc *est.RPC) {
 	g.Comment("Serialize the response")
-	g.Var().Id("respData").Index().Byte()
 	resp, err := encoding.DescribeResponse(b.res.Meta, rpc.Response.Type, nil)
 	if err != nil {
 		b.errors.Addf(rpc.Func.Pos(), "failed to describe response: %v", err.Error())
 	}
 
 	if len(resp.BodyParameters) > 0 {
-		g.Line().Comment("Encode JSON body")
-		g.List(Id("respData"), Err()).Op("=").Qual("encore.dev/runtime/serde", "SerializeJSONFunc").Call(Id("json"), Func().Params(Id("ser").Op("*").Qual("encore.dev/runtime/serde", "JSONSerializer")).BlockFunc(
-			func(g *Group) {
-				for _, f := range resp.BodyParameters {
-					g.Add(Id("ser").Dot("WriteField").Call(Lit(f.Name), Id("resp").Dot(f.SrcName), Lit(f.OmitEmpty)))
-				}
-			}))
-		g.If(Err().Op("!=").Nil()).Block(
-			Id("marshalErr").Op(":=").Add(wrapErrCode(Err(), "Internal", "failed to marshal response")),
-			Qual("encore.dev/runtime", "FinishRequest").Call(Nil(), Id("marshalErr")),
-			Qual("encore.dev/beta/errs", "HTTPError").Call(Id("w"), Id("marshalErr")),
-			Return(),
-		)
+		g.Id("respData").Op(":=").Index().Byte().Parens(Lit("null\n"))
+	} else {
+		g.Id("respData").Op(":=").Index().Byte().Values(LitRune('\n'))
 	}
-
 	if len(resp.HeaderParameters) > 0 {
-		headerEncoder := b.marshaller.NewPossibleInstance("headerEncoder")
-		g.Line().Comment("Encode headers")
-		headerEncoder.Add(Id("headers").Op(":=").Map(String()).Index().String().ValuesFunc(
-			func(g *Group) {
-				for _, f := range resp.HeaderParameters {
-					headerSlice, err := headerEncoder.ToStringSlice(f.Type, Id("resp").Dot(f.SrcName))
-					if err != nil {
-						b.errors.Addf(rpc.Func.Pos(), "failed to generate haader serializers: %v", err.Error())
+		g.Var().Id("headers").Map(String()).Index().String()
+	}
+
+	responseEncoder := CustomFunc(Options{Separator: "\n"}, func(g *Group) {
+		if len(resp.BodyParameters) > 0 {
+			g.Comment("Encode JSON body")
+			g.List(Id("respData"), Err()).Op("=").Qual("encore.dev/runtime/serde", "SerializeJSONFunc").Call(Id("json"), Func().Params(Id("ser").Op("*").Qual("encore.dev/runtime/serde", "JSONSerializer")).BlockFunc(
+				func(g *Group) {
+					for _, f := range resp.BodyParameters {
+						g.Add(Id("ser").Dot("WriteField").Call(Lit(f.Name), Id("resp").Dot(f.SrcName), Lit(f.OmitEmpty)))
 					}
-					g.Add(Lit(f.Name).Op(":").Add(headerSlice))
-				}
-			}))
-		g.Add(headerEncoder.Finalize(
-			Id("headerErr").Op(":=").Add(wrapErrCode(Id("headerEncoder").Dot("LastError"), "Internal", "failed to marshal headers")),
-			Qual("encore.dev/runtime", "FinishRequest").Call(Nil(), Id("headerErr")),
-			Qual("encore.dev/beta/errs", "HTTPError").Call(Id("w"), Id("headerErr")),
-			Return(),
-		)...)
+				}))
+			g.If(Err().Op("!=").Nil()).Block(
+				Id("marshalErr").Op(":=").Add(wrapErrCode(Err(), "Internal", "failed to marshal response")),
+				Qual("encore.dev/runtime", "FinishRequest").Call(Nil(), Id("marshalErr")),
+				Qual("encore.dev/beta/errs", "HTTPError").Call(Id("w"), Id("marshalErr")),
+				Return(),
+			)
+			g.Id("respData").Op("=").Append(Id("respData"), LitRune('\n'))
+		}
+
+		if len(resp.HeaderParameters) > 0 {
+			headerEncoder := b.marshaller.NewPossibleInstance("headerEncoder")
+			g.Line().Comment("Encode headers")
+			headerEncoder.Add(Id("headers").Op("=").Map(String()).Index().String().ValuesFunc(
+				func(g *Group) {
+					for _, f := range resp.HeaderParameters {
+						headerSlice, err := headerEncoder.ToStringSlice(f.Type, Id("resp").Dot(f.SrcName))
+						if err != nil {
+							b.errors.Addf(rpc.Func.Pos(), "failed to generate haader serializers: %v", err.Error())
+						}
+						g.Add(Lit(f.Name).Op(":").Add(headerSlice))
+					}
+				}))
+			g.Add(headerEncoder.Finalize(
+				Id("headerErr").Op(":=").Add(wrapErrCode(Id("headerEncoder").Dot("LastError"), "Internal", "failed to marshal headers")),
+				Qual("encore.dev/runtime", "FinishRequest").Call(Nil(), Id("headerErr")),
+				Qual("encore.dev/beta/errs", "HTTPError").Call(Id("w"), Id("headerErr")),
+				Return(),
+			)...)
+		}
+	})
+
+	// If response is a ptr we need to check it's not nil
+	if rpc.Response.IsPtr {
+		g.If(Id("resp").Op("!=").Nil()).Block(responseEncoder)
+	} else {
+		g.Add(responseEncoder)
 	}
 
 	g.Line().Comment("Record tracing data")
-	g.Id("respData").Op("=").Append(Id("respData"), LitRune('\n'))
 	g.Id("output").Op(":=").Index().Index().Byte().Values(Id("respData"))
 	g.Qual("encore.dev/runtime", "FinishRequest").Call(Id("output"), Nil())
 

diff --git a/compiler/internal/codegen/testdata/TestCodeGenMain__token_auth.golden b/compiler/internal/codegen/testdata/TestCodeGenMain__token_auth.golden
@@ -123,21 +123,22 @@ func __encore_svc_Eight(w http.ResponseWriter, req *http.Request, ps httprouter.
 	}
 
 	// Serialize the response
-	var respData []byte
-
-	// Encode JSON body
-	respData, err = serde.SerializeJSONFunc(json, func(ser *serde.JSONSerializer) {
-		ser.WriteField("Message", resp.Message, false)
-	})
-	if err != nil {
-		marshalErr := errs.WrapCode(err, errs.Internal, "failed to marshal response")
-		runtime.FinishRequest(nil, marshalErr)
-		errs.HTTPError(w, marshalErr)
-		return
+	respData := []byte("null\n")
+	if resp != nil {
+		// Encode JSON body
+		respData, err = serde.SerializeJSONFunc(json, func(ser *serde.JSONSerializer) {
+			ser.WriteField("Message", resp.Message, false)
+		})
+		if err != nil {
+			marshalErr := errs.WrapCode(err, errs.Internal, "failed to marshal response")
+			runtime.FinishRequest(nil, marshalErr)
+			errs.HTTPError(w, marshalErr)
+			return
+		}
+		respData = append(respData, '\n')
 	}
 
 	// Record tracing data
-	respData = append(respData, '\n')
 	output := [][]byte{respData}
 	runtime.FinishRequest(output, nil)