fixing error handling of unfound block (#15742)

beacon-chain/rpc/eth/beacon/handlers.go

@@ -1465,8 +1465,7 @@ func (s *Server) GetBlockHeader(w http.ResponseWriter, r *http.Request) {
 	}
 
 	blk, err := s.Blocker.Block(ctx, []byte(blockID))
-	ok := shared.WriteBlockFetchError(w, blk, err)
-	if !ok {
+	if !shared.WriteBlockFetchError(w, blk, err) {
 		return
 	}
 	blockHeader, err := blk.Header()

beacon-chain/rpc/eth/blob/handlers.go

@@ -58,12 +58,7 @@ func (s *Server) Blobs(w http.ResponseWriter, r *http.Request) {
 	}
 
 	blk, err := s.Blocker.Block(ctx, []byte(blockId))
-	if err != nil {
-		httputil.HandleError(w, "Could not fetch block: "+err.Error(), http.StatusInternalServerError)
-		return
-	}
-	if blk == nil {
-		httputil.HandleError(w, "Block not found", http.StatusNotFound)
+	if !shared.WriteBlockFetchError(w, blk, err) {
 		return
 	}
 
@@ -174,12 +169,7 @@ func (s *Server) GetBlobs(w http.ResponseWriter, r *http.Request) {
 	}
 
 	blk, err := s.Blocker.Block(ctx, []byte(blockId))
-	if err != nil {
-		httputil.HandleError(w, "Could not fetch block: "+err.Error(), http.StatusInternalServerError)
-		return
-	}
-	if blk == nil {
-		httputil.HandleError(w, "Block not found", http.StatusNotFound)
+	if !shared.WriteBlockFetchError(w, blk, err) {
 		return
 	}
 

beacon-chain/rpc/eth/debug/handlers.go

@@ -269,12 +269,7 @@ func (s *Server) DataColumnSidecars(w http.ResponseWriter, r *http.Request) {
 	}
 
 	blk, err := s.Blocker.Block(ctx, []byte(blockId))
-	if err != nil {
-		httputil.HandleError(w, "Could not fetch block: "+err.Error(), http.StatusInternalServerError)
-		return
-	}
-	if blk == nil {
-		httputil.HandleError(w, "Block not found", http.StatusNotFound)
+	if !shared.WriteBlockFetchError(w, blk, err) {
 		return
 	}
 

beacon-chain/rpc/eth/shared/errors.go

@@ -29,6 +29,11 @@ func WriteStateFetchError(w http.ResponseWriter, err error) {
 // WriteBlockFetchError writes an appropriate error based on the supplied argument.
 // The argument error should be a result of fetching block.
 func WriteBlockFetchError(w http.ResponseWriter, blk interfaces.ReadOnlySignedBeaconBlock, err error) bool {
+	var blockNotFoundErr *lookup.BlockNotFoundError
+	if errors.As(err, &blockNotFoundErr) {
+		httputil.HandleError(w, "Block not found: "+blockNotFoundErr.Error(), http.StatusNotFound)
+		return false
+	}
 	var invalidBlockIdErr *lookup.BlockIdParseError
 	if errors.As(err, &invalidBlockIdErr) {
 		httputil.HandleError(w, "Invalid block ID: "+invalidBlockIdErr.Error(), http.StatusBadRequest)

beacon-chain/rpc/eth/shared/errors_test.go

@@ -49,3 +49,59 @@ func TestWriteStateFetchError(t *testing.T) {
 		assert.NoError(t, json.Unmarshal(writer.Body.Bytes(), e), "failed to unmarshal response")
 	}
 }
+
+// TestWriteBlockFetchError tests the WriteBlockFetchError function
+// to ensure that the correct error message and code are written to the response
+// and that the function returns the correct boolean value.
+func TestWriteBlockFetchError(t *testing.T) {
+	cases := []struct {
+		name            string
+		err             error
+		expectedMessage string
+		expectedCode    int
+		expectedReturn  bool
+	}{
+		{
+			name:            "BlockNotFoundError should return 404",
+			err:             lookup.NewBlockNotFoundError("block not found at slot 123"),
+			expectedMessage: "Block not found",
+			expectedCode:    http.StatusNotFound,
+			expectedReturn:  false,
+		},
+		{
+			name:            "BlockIdParseError should return 400",
+			err:             &lookup.BlockIdParseError{},
+			expectedMessage: "Invalid block ID",
+			expectedCode:    http.StatusBadRequest,
+			expectedReturn:  false,
+		},
+		{
+			name:            "Generic error should return 500",
+			err:             errors.New("database connection failed"),
+			expectedMessage: "Could not get block from block ID",
+			expectedCode:    http.StatusInternalServerError,
+			expectedReturn:  false,
+		},
+		{
+			name:            "Nil block should return 404",
+			err:             nil,
+			expectedMessage: "Could not find requested block",
+			expectedCode:    http.StatusNotFound,
+			expectedReturn:  false,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			writer := httptest.NewRecorder()
+			result := WriteBlockFetchError(writer, nil, c.err)
+
+			assert.Equal(t, c.expectedReturn, result, "incorrect return value")
+			assert.Equal(t, c.expectedCode, writer.Code, "incorrect status code")
+			assert.StringContains(t, c.expectedMessage, writer.Body.String(), "incorrect error message")
+
+			e := &httputil.DefaultJsonError{}
+			assert.NoError(t, json.Unmarshal(writer.Body.Bytes(), e), "failed to unmarshal response")
+		})
+	}
+}

changelog/james_prysm-fix-blocker-notfound.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- fixed regression introduced in PR #15715 , blocker now returns an error for not found, and error handling correctly handles error and returns 404 instead of 500