From c46884aa93e62bb39fd084433d840db4543b7792 Mon Sep 17 00:00:00 2001
From: Nalin Dahyabhai <nalin@redhat.com>
Date: Mon, 13 May 2024 13:29:09 -0400
Subject: [PATCH] `podman events`: check for an error after we finish reading
 events

The function that's handing us events will return an error after closing
the channel over which it's sending events, and its caller (in its own
goroutine) will then send that error over another channel.

The logic that started the goroutine is likely to notice that the events
channel is closed before noticing that the error channel has a result
for it to read, so any error that would have been communicated would be
lost.

When we finish reading events, check if the reader returned an error
before telling our caller that there was no error.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
---
 cmd/podman/system/events.go       | 10 ++++++++--
 pkg/api/handlers/compat/events.go | 14 ++++++++++----
 pkg/bindings/system/system.go     |  4 ++++
 test/system/090-events.bats       |  5 +++++
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/cmd/podman/system/events.go b/cmd/podman/system/events.go
index a7c733df1f..44b24319c0 100644
--- a/cmd/podman/system/events.go
+++ b/cmd/podman/system/events.go
@@ -162,8 +162,7 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 	}
 
 	go func() {
-		err := registry.ContainerEngine().Events(context.Background(), eventOptions)
-		errChannel <- err
+		errChannel <- registry.ContainerEngine().Events(context.Background(), eventOptions)
 	}()
 
 	for {
@@ -171,6 +170,13 @@ func eventsCmd(cmd *cobra.Command, _ []string) error {
 		case event, ok := <-eventChannel:
 			if !ok {
 				// channel was closed we can exit
+				select {
+				case err := <-errChannel:
+					if err != nil {
+						return err
+					}
+				default:
+				}
 				return nil
 			}
 			switch {
diff --git a/pkg/api/handlers/compat/events.go b/pkg/api/handlers/compat/events.go
index ceb1493e03..4ab12eb022 100644
--- a/pkg/api/handlers/compat/events.go
+++ b/pkg/api/handlers/compat/events.go
@@ -43,7 +43,7 @@ func GetEvents(w http.ResponseWriter, r *http.Request) {
 
 	libpodFilters, err := util.FiltersFromRequest(r)
 	if err != nil {
-		utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err))
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse filters for %s: %w", r.URL.String(), err))
 		return
 	}
 	eventChannel := make(chan *events.Event)
@@ -68,8 +68,13 @@ func GetEvents(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(http.StatusOK)
-	flush()
+	wroteContent := false
+	defer func() {
+		if !wroteContent {
+			w.WriteHeader(http.StatusOK)
+			flush()
+		}
+	}()
 
 	coder := json.NewEncoder(w)
 	coder.SetEscapeHTML(true)
@@ -78,8 +83,8 @@ func GetEvents(w http.ResponseWriter, r *http.Request) {
 		select {
 		case err := <-errorChannel:
 			if err != nil {
-				// FIXME StatusOK already sent above cannot send 500 here
 				utils.InternalServerError(w, err)
+				wroteContent = true
 			}
 			return
 		case evt := <-eventChannel:
@@ -103,6 +108,7 @@ func GetEvents(w http.ResponseWriter, r *http.Request) {
 			if err := coder.Encode(e); err != nil {
 				logrus.Errorf("Unable to write json: %q", err)
 			}
+			wroteContent = true
 			flush()
 		case <-r.Context().Done():
 			return
diff --git a/pkg/bindings/system/system.go b/pkg/bindings/system/system.go
index e97ebc7b66..dbe5c318ea 100644
--- a/pkg/bindings/system/system.go
+++ b/pkg/bindings/system/system.go
@@ -42,6 +42,10 @@ func Events(ctx context.Context, eventChan chan types.Event, cancelChan chan boo
 		}()
 	}
 
+	if response.StatusCode != http.StatusOK {
+		return response.Process(nil)
+	}
+
 	dec := json.NewDecoder(response.Body)
 	for err = (error)(nil); err == nil; {
 		var e = types.Event{}
diff --git a/test/system/090-events.bats b/test/system/090-events.bats
index 5f023400b3..072d7f9f4f 100644
--- a/test/system/090-events.bats
+++ b/test/system/090-events.bats
@@ -406,3 +406,8 @@ EOF
     run_podman events --since=1m --stream=false --filter volume=${vname:0:5}
     assert "$output" = "$notrunc_results"
 }
+
+@test "events - invalid filter" {
+    run_podman 125 events --since="the dawn of time...ish"
+    assert "$output" =~ "failed to parse event filters"
+}