From 01329987f3c3570974715e12283b4521717824ab Mon Sep 17 00:00:00 2001 From: Gabriel Paradiso Date: Thu, 19 Dec 2024 12:13:35 +0100 Subject: [PATCH 1/2] fix: do not fail when deleting non existing workflow spec --- core/services/workflows/syncer/handler.go | 4 +- .../services/workflows/syncer/handler_test.go | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/core/services/workflows/syncer/handler.go b/core/services/workflows/syncer/handler.go index ef5455bfbc8..a4be3a69c29 100644 --- a/core/services/workflows/syncer/handler.go +++ b/core/services/workflows/syncer/handler.go @@ -3,6 +3,7 @@ package syncer import ( "bytes" "context" + "database/sql" "encoding/base64" "encoding/hex" "encoding/json" @@ -642,7 +643,8 @@ func (h *eventHandler) workflowDeletedEvent( return err } - if err := h.orm.DeleteWorkflowSpec(ctx, hex.EncodeToString(payload.WorkflowOwner), payload.WorkflowName); err != nil { + err := h.orm.DeleteWorkflowSpec(ctx, hex.EncodeToString(payload.WorkflowOwner), payload.WorkflowName) + if err != nil && !errors.Is(err, sql.ErrNoRows) { return fmt.Errorf("failed to delete workflow spec: %w", err) } return nil diff --git a/core/services/workflows/syncer/handler_test.go b/core/services/workflows/syncer/handler_test.go index f205cbde1cd..994b820b5ce 100644 --- a/core/services/workflows/syncer/handler_test.go +++ b/core/services/workflows/syncer/handler_test.go @@ -555,6 +555,61 @@ func Test_workflowDeletedHandler(t *testing.T) { _, err = h.engineRegistry.Get(wfIDs) require.Error(t, err) }) + t.Run("success deleting non-existing workflow spec", func(t *testing.T) { + var ( + ctx = testutils.Context(t) + lggr = logger.TestLogger(t) + db = pgtest.NewSqlxDB(t) + orm = NewWorkflowRegistryDS(db, lggr) + emitter = custmsg.NewLabeler() + + binary = wasmtest.CreateTestBinary(binaryCmd, binaryLocation, true, t) + encodedBinary = []byte(base64.StdEncoding.EncodeToString(binary)) + config = []byte("") + secretsURL = "http://example.com" + binaryURL = "http://example.com/binary" + configURL = "http://example.com/config" + wfOwner = []byte("0xOwner") + + fetcher = newMockFetcher(map[string]mockFetchResp{ + binaryURL: {Body: encodedBinary, Err: nil}, + configURL: {Body: config, Err: nil}, + secretsURL: {Body: []byte("secrets"), Err: nil}, + }) + ) + + giveWFID, err := pkgworkflows.GenerateWorkflowID(wfOwner, "workflow-name", binary, config, secretsURL) + require.NoError(t, err) + + er := NewEngineRegistry() + store := wfstore.NewDBStore(db, lggr, clockwork.NewFakeClock()) + registry := capabilities.NewRegistry(lggr) + registry.SetLocalRegistry(&capabilities.TestMetadataRegistry{}) + h := NewEventHandler( + lggr, + orm, + fetcher, + store, + registry, + emitter, + clockwork.NewFakeClock(), + workflowkey.Key{}, + WithEngineRegistry(er), + ) + + deleteEvent := WorkflowRegistryWorkflowDeletedV1{ + WorkflowID: giveWFID, + WorkflowOwner: wfOwner, + WorkflowName: "workflow-name", + DonID: 1, + } + err = h.workflowDeletedEvent(ctx, deleteEvent) + require.NoError(t, err) + + // Verify the record is deleted in the database + _, err = orm.GetWorkflowSpec(ctx, hex.EncodeToString(wfOwner), "workflow-name") + require.Error(t, err) + }) } func Test_workflowPausedActivatedUpdatedHandler(t *testing.T) { From 765e57060dab1a30a46b7dc5816d9761c4637a54 Mon Sep 17 00:00:00 2001 From: Gabriel Paradiso Date: Thu, 19 Dec 2024 12:25:34 +0100 Subject: [PATCH 2/2] chore: add warning if workflow is not found --- core/services/workflows/syncer/handler.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/services/workflows/syncer/handler.go b/core/services/workflows/syncer/handler.go index a4be3a69c29..cb4f013d502 100644 --- a/core/services/workflows/syncer/handler.go +++ b/core/services/workflows/syncer/handler.go @@ -644,9 +644,14 @@ func (h *eventHandler) workflowDeletedEvent( } err := h.orm.DeleteWorkflowSpec(ctx, hex.EncodeToString(payload.WorkflowOwner), payload.WorkflowName) - if err != nil && !errors.Is(err, sql.ErrNoRows) { + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + h.lggr.Warnw("workflow spec not found", "workflowID", hex.EncodeToString(payload.WorkflowID[:])) + return nil + } return fmt.Errorf("failed to delete workflow spec: %w", err) } + return nil }