From 3629bcf057c3782f732131452bd58d4d12db9544 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 15:01:51 +0100 Subject: [PATCH] Read only gitea's 404 status as not-found, so a 502 naming a sha with 404 in it is redelivered, not written backwards (issue 1126, review) --- .../internal/index/modules_test.go | 15 +++++++++++++++ .../mesh-delivery-indexer/internal/index/order.go | 7 +++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/index/modules_test.go b/modules/mesh-delivery-indexer/internal/index/modules_test.go index 6027366..e65bc31 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules_test.go +++ b/modules/mesh-delivery-indexer/internal/index/modules_test.go @@ -207,6 +207,21 @@ func TestAnUnchangedModuleWritesNothing(t *testing.T) { // 1239 (unchanged value } return ix.Apply(context.Background(), mergedMsg(repo, three)) }}, + // Review round 2: a 502 whose words carry "404" inside a sha is no not-found; the late message is left for redelivery. + {name: "a late message whose order the forge fails with a 502 writes nothing and is left for redelivery", writes: 1, commit: map[string]string{"a": next, "b": merge, "c": merge}, taken: next, err: true, + said: "502 Bad Gateway", + then: func(ix *Indexer) error { + ix.Contains = func(context.Context, string, string, string) (bool, error) { return false, nil } + later := mergedMsg(repo, map[string]string{"modules/a": "treeA2", "modules/b": "treeB", "modules/c": "treeC"}) + later.Merge = next + if err := ix.Apply(context.Background(), later); err != nil { + return err + } + ix.Contains = func(context.Context, string, string, string) (bool, error) { + return false, errors.New("Gitea API /repos/x/y/compare/aaa404aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb: 502 Bad Gateway") + } + return ix.Apply(context.Background(), mergedMsg(repo, three)) + }}, {name: "a forge that cannot say the order writes nothing and leaves the message for redelivery", writes: 0, commit: map[string]string{"a": merge}, taken: merge, err: true, said: "cannot be read", then: func(ix *Indexer) error { diff --git a/modules/mesh-delivery-indexer/internal/index/order.go b/modules/mesh-delivery-indexer/internal/index/order.go index d9044dd..c09cc5e 100644 --- a/modules/mesh-delivery-indexer/internal/index/order.go +++ b/modules/mesh-delivery-indexer/internal/index/order.go @@ -40,8 +40,11 @@ func newMergeWrite(f delivery.Forge) *mergeWrite { return &mergeWrite{repository: f.Repository, merge: f.Merge} } -// notFound says the forge answered that a commit it was asked about does not exist (gitea's 404, as readVia reads it). -func notFound(err error) bool { return err != nil && strings.Contains(err.Error(), "404") } +// notFound says the forge answered that a commit it was asked about does not exist: gitea's status ": 404 ", matched as +// readVia matches it (cmd/mesh-delivery-indexer named.go). A bare "404" is not enough: the error's words carry both shas, +// and a 502 naming a sha with 404 in it would read as not older and write a late message backwards. The thorough form, +// a typed not-found answered by gitea_contains, is noted on issue 1126. +func notFound(err error) bool { return err != nil && strings.Contains(err.Error(), ": 404 ") } // older says commit is older than later: the forge says commit is in later and the two differ. A commit the forge does not // have is not known to be older (false, said); a forge that cannot say otherwise is an error. Without a forge to ask