From 3ea2e4734dc91a129de5faf837c831a50fe1cbc7 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Tue, 13 Feb 2024 17:23:08 -0700 Subject: [PATCH] isolate the actual error from this query I couldn't the exact error, so this is the best approximation i could come up with. --- .../m365/collection/exchange/events_backup.go | 13 +++++++++-- .../collection/exchange/events_backup_test.go | 22 ++++++++++++++++--- src/pkg/services/m365/api/graph/errors.go | 3 ++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/internal/m365/collection/exchange/events_backup.go b/src/internal/m365/collection/exchange/events_backup.go index 617fb474a..5a1f473c1 100644 --- a/src/internal/m365/collection/exchange/events_backup.go +++ b/src/internal/m365/collection/exchange/events_backup.go @@ -2,6 +2,7 @@ package exchange import ( "errors" + "net/http" "slices" "github.com/alcionai/clues" @@ -60,13 +61,21 @@ func (h eventBackupHandler) NewContainerCache( } } +// todo: this could be further improved buy specifying the call source and matching that +// with the expected error. Might be necessary if we use this for more than one error. +// But since we only call this in a single place at this time, that additional guard isn't +// built into the func. func (h eventBackupHandler) CanSkipItemFailure( err error, resourceID, itemID string, opts control.Options, ) (fault.SkipCause, bool) { - // yes, this is intentionally a todo. I'll get back to it. - if !errors.Is(err, clues.New("todo fix me")) { + if err == nil { + return "", false + } + + if !errors.Is(err, graph.ErrServiceUnavailableEmptyResp) && + !clues.HasLabel(err, graph.LabelStatus(http.StatusServiceUnavailable)) { return "", false } diff --git a/src/internal/m365/collection/exchange/events_backup_test.go b/src/internal/m365/collection/exchange/events_backup_test.go index 853d6bda9..f8ef78f7b 100644 --- a/src/internal/m365/collection/exchange/events_backup_test.go +++ b/src/internal/m365/collection/exchange/events_backup_test.go @@ -1,6 +1,7 @@ package exchange import ( + "net/http" "testing" "github.com/alcionai/clues" @@ -12,6 +13,7 @@ import ( "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/services/m365/api" + "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) type EventsBackupHandlerUnitSuite struct { @@ -61,7 +63,7 @@ func (suite *EventsBackupHandlerUnitSuite) TestHandler_CanSkipItemFailure() { }, { name: "non-matching resource", - err: clues.New("fix me I'm wrong"), + err: graph.ErrServiceUnavailableEmptyResp, opts: control.Options{ SkipTheseEventsOnInstance503: map[string][]string{ "foo": {"bar", "baz"}, @@ -71,17 +73,31 @@ func (suite *EventsBackupHandlerUnitSuite) TestHandler_CanSkipItemFailure() { }, { name: "non-matching item", - err: clues.New("fix me I'm wrong"), + err: graph.ErrServiceUnavailableEmptyResp, opts: control.Options{ SkipTheseEventsOnInstance503: map[string][]string{ resourceID: {"bar", "baz"}, }, }, expect: assert.False, + // the item won't match, but we still return this as the cause + expectCause: fault.SkipKnownEventInstance503s, + }, + { + name: "match on instance 503 empty resp", + err: graph.ErrServiceUnavailableEmptyResp, + opts: control.Options{ + SkipTheseEventsOnInstance503: map[string][]string{ + resourceID: {"bar", itemID}, + }, + }, + expect: assert.True, + expectCause: fault.SkipKnownEventInstance503s, }, { name: "match on instance 503", - err: clues.New("fix me I'm wrong"), + err: clues.New("arbitrary error"). + Label(graph.LabelStatus(http.StatusServiceUnavailable)), opts: control.Options{ SkipTheseEventsOnInstance503: map[string][]string{ resourceID: {"bar", itemID}, diff --git a/src/pkg/services/m365/api/graph/errors.go b/src/pkg/services/m365/api/graph/errors.go index 0b7793adb..b3bd4b740 100644 --- a/src/pkg/services/m365/api/graph/errors.go +++ b/src/pkg/services/m365/api/graph/errors.go @@ -156,7 +156,8 @@ func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error { labels = append(labels, core.LabelRootCauseUnknown) } - stacked := stackWithDepth(ctx, err, 1+traceDepth) + stacked := stackWithDepth(ctx, err, 1+traceDepth). + Label(LabelStatus(ode.Resp.StatusCode)) // labeling here because we want the context from stackWithDepth first for _, label := range labels {