clean up all elems on error

Author: linkdata

ui/containerhelper.go

@@ -49,15 +49,20 @@ func (ui *ContainerHelper) RenderContainer(e *core.Element, w io.Writer, outerHT
 			var contents []*core.Element
 			for _, childUI := range ui.Container.JawsContains(e) {
 				elem := e.Request.NewElement(childUI)
+				contents = append(contents, elem)
 				if err = elem.JawsRender(w, nil); err != nil {
-					e.Request.DeleteElement(elem)
 					break
 				}
-				contents = append(contents, elem)
 			}
-			ui.mu.Lock()
-			ui.contents = contents
-			ui.mu.Unlock()
+			if err == nil {
+				ui.mu.Lock()
+				ui.contents = contents
+				ui.mu.Unlock()
+			} else {
+				for _, elem := range contents {
+					e.Request.DeleteElement(elem)
+				}
+			}
 			b = b[:0]
 			b = append(b, "</"...)
 			b = append(b, outerHTMLTag...)

ui/containerhelper_test.go

@@ -133,8 +133,8 @@ func TestContainerHelperRenderErrorPaths(t *testing.T) {
 	if !errors.Is(err, renderErr) {
 		t.Fatalf("want %v got %v", renderErr, err)
 	}
-	if len(container.contents) != 1 {
-		t.Fatalf("want 1 successful child got %d", len(container.contents))
+	if len(container.contents) != 0 {
+		t.Fatalf("want 0 successful child got %d", len(container.contents))
 	}
 
 	// panic path from must() during append
@@ -195,6 +195,29 @@ func TestContainerHelperRenderErrorDoesNotLeakFailedChildElement(t *testing.T) {
 	}
 }
 
+func TestRequestWriterUI_ContainerRenderErrorDoesNotLeakSuccessfulChildren(t *testing.T) {
+	core.NextJid = 0
+	_, rq := newRequest(t)
+	var sb strings.Builder
+	rw := RequestWriter{Request: rq, Writer: &sb}
+
+	renderErr := errors.New("render error")
+	okChild := &testRenderErrorCaptureUI{}
+	failChild := &testRenderErrorCaptureUI{err: renderErr}
+	tc := &testContainer{contents: []core.UI{okChild, failChild}}
+
+	if err := rw.UI(NewContainer("div", tc)); !errors.Is(err, renderErr) {
+		t.Fatalf("want %v got %v", renderErr, err)
+	}
+
+	if !okChild.jid.IsValid() {
+		t.Fatal("expected successful child jid to be captured")
+	}
+	if leaked := rq.GetElementByJid(okChild.jid); leaked != nil {
+		t.Fatalf("expected successful child %v to be removed when parent render fails", okChild.jid)
+	}
+}
+
 type testSelectHandler struct {
 	*testContainer
 	*testSetter[string]