Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Commit 7afe06a

Browse files
chkeitachkeita
authored
Return the failure from NotifyAdo as result instead of an exception (#3555)
* Return the failure from NotifyAdo as result instead of an exception * remove comment * requeue when notification fails * fix logic
1 parent d792b3b commit 7afe06a

File tree

7 files changed

+29
-28
lines changed

7 files changed

+29
-28
lines changed

‎src/ApiService/ApiService/Functions/NotificationsTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public async Async.Task<HttpResponseData> Run([HttpTrigger(AuthorizationLevel.An
3030
}
3131

3232
var result = await _context.NotificationOperations.TriggerNotification(notificationTest.Notification.Container, notificationTest.Notification,
33-
notificationTest.Report, isLastRetryAttempt: true);
33+
notificationTest.Report);
3434
var response = req.CreateResponse(HttpStatusCode.OK);
3535
await response.WriteAsJsonAsync(new NotificationTestResponse(result.IsOk, result.ErrorV?.ToString()));
3636
return response;

‎src/ApiService/ApiService/Functions/QueueFileChanges.cs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,17 @@ public async Async.Task Run(
5858
var storageAccount = new ResourceIdentifier(topicElement.GetString()!);
5959

6060
try {
61-
// Setting isLastRetryAttempt to false will rethrow any exceptions
62-
// With the intention that the azure functions runtime will handle requeing
63-
// the message for us. The difference is for the poison queue, we're handling the
64-
// requeuing ourselves because azure functions doesn't support retry policies
65-
// for queue based functions.
66-
67-
var result = await FileAdded(storageAccount, fileChangeEvent, isLastRetryAttempt: false);
68-
if (!result.IsOk && result.ErrorV.Code == ErrorCode.ADO_WORKITEM_PROCESSING_DISABLED) {
69-
await RequeueMessage(msg, TimeSpan.FromDays(1));
61+
var result = await FileAdded(storageAccount, fileChangeEvent);
62+
if (!result.IsOk) {
63+
await RequeueMessage(msg, result.ErrorV.Code == ErrorCode.ADO_WORKITEM_PROCESSING_DISABLED ? TimeSpan.FromDays(1) : null);
7064
}
7165
} catch (Exception e) {
7266
_log.LogError(e, "File Added failed");
7367
await RequeueMessage(msg);
7468
}
7569
}
7670

77-
private async Async.Task<OneFuzzResultVoid> FileAdded(ResourceIdentifier storageAccount, JsonDocument fileChangeEvent, bool isLastRetryAttempt) {
71+
private async Async.Task<OneFuzzResultVoid> FileAdded(ResourceIdentifier storageAccount, JsonDocument fileChangeEvent) {
7872
var data = fileChangeEvent.RootElement.GetProperty("data");
7973
var url = data.GetProperty("url").GetString()!;
8074
var parts = url.Split("/").Skip(3).ToList();
@@ -86,7 +80,7 @@ private async Async.Task<OneFuzzResultVoid> FileAdded(ResourceIdentifier storage
8680

8781
var (_, result) = await (
8882
ApplyRetentionPolicy(storageAccount, container, path),
89-
_notificationOperations.NewFiles(container, path, isLastRetryAttempt));
83+
_notificationOperations.NewFiles(container, path));
9084

9185
return result;
9286
}

‎src/ApiService/ApiService/OneFuzzTypes/Enums.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public enum ErrorCode {
5252
ADO_VALIDATION_INVALID_PROJECT = 496,
5353
INVALID_RETENTION_PERIOD = 497,
5454
INVALID_CLI_VERSION = 498,
55+
TRANSIENT_NOTIFICATION_FAILURE = 499,
5556
// NB: if you update this enum, also update enums.py
5657
}
5758

‎src/ApiService/ApiService/TestHooks/NotificationOperationsTestHooks.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ public async Task<HttpResponseData> NewFiles([HttpTrigger(AuthorizationLevel.Ano
2929

3030
var container = query["container"];
3131
var fileName = query["fileName"];
32-
var isLastRetryAttempt = UriExtension.GetBool("isLastRetryAttempt", query, true);
33-
34-
_ = await _notificationOps.NewFiles(Container.Parse(container), fileName, isLastRetryAttempt);
32+
_ = await _notificationOps.NewFiles(Container.Parse(container), fileName);
3533
var resp = req.CreateResponse(HttpStatusCode.OK);
3634
return resp;
3735
}

‎src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
namespace Microsoft.OneFuzz.Service;
66

77
public interface INotificationOperations : IOrm<Notification> {
8-
Async.Task<OneFuzzResultVoid> NewFiles(Container container, string filename, bool isLastRetryAttempt);
8+
Async.Task<OneFuzzResultVoid> NewFiles(Container container, string filename);
99
IAsyncEnumerable<Notification> GetNotifications(Container container);
1010
IAsyncEnumerable<(Task, IEnumerable<Container>)> GetQueueTasks();
1111
Async.Task<OneFuzzResult<Notification>> Create(Container container, NotificationTemplate config, bool replaceExisting);
1212
Async.Task<Notification?> GetNotification(Guid notifificationId);
1313

1414
System.Threading.Tasks.Task<OneFuzzResultVoid> TriggerNotification(Container container,
15-
Notification notification, IReport? reportOrRegression, bool isLastRetryAttempt = false);
15+
Notification notification, IReport? reportOrRegression);
1616
}
1717

1818
public class NotificationOperations : Orm<Notification>, INotificationOperations {
@@ -21,7 +21,7 @@ public NotificationOperations(ILogger<NotificationOperations> log, IOnefuzzConte
2121
: base(log, context) {
2222

2323
}
24-
public async Async.Task<OneFuzzResultVoid> NewFiles(Container container, string filename, bool isLastRetryAttempt) {
24+
public async Async.Task<OneFuzzResultVoid> NewFiles(Container container, string filename) {
2525
// We don't want to store file added events for the events container because that causes an infinite loop
2626
if (container == WellKnownContainers.Events) {
2727
return Result.Ok();
@@ -39,7 +39,7 @@ public async Async.Task<OneFuzzResultVoid> NewFiles(Container container, string
3939
}
4040

4141
done.Add(notification.Config);
42-
var notificationResult = await TriggerNotification(container, notification, reportOrRegression, isLastRetryAttempt);
42+
var notificationResult = await TriggerNotification(container, notification, reportOrRegression);
4343
if (result.IsOk && !notificationResult.IsOk) {
4444
result = notificationResult;
4545
}
@@ -86,15 +86,15 @@ public async Async.Task<OneFuzzResultVoid> NewFiles(Container container, string
8686
}
8787

8888
public async System.Threading.Tasks.Task<OneFuzzResultVoid> TriggerNotification(Container container,
89-
Notification notification, IReport? reportOrRegression, bool isLastRetryAttempt = false) {
89+
Notification notification, IReport? reportOrRegression) {
9090
switch (notification.Config) {
9191
case TeamsTemplate teamsTemplate:
9292
await _context.Teams.NotifyTeams(teamsTemplate, container, reportOrRegression!,
9393
notification.NotificationId);
9494
break;
9595
case AdoTemplate adoTemplate when reportOrRegression is not null:
9696
if (await _context.FeatureManagerSnapshot.IsEnabledAsync(FeatureFlagConstants.EnableWorkItemCreation)) {
97-
return await _context.Ado.NotifyAdo(adoTemplate, container, reportOrRegression, isLastRetryAttempt,
97+
return await _context.Ado.NotifyAdo(adoTemplate, container, reportOrRegression,
9898
notification.NotificationId);
9999
} else {
100100
return OneFuzzResultVoid.Error(ErrorCode.ADO_WORKITEM_PROCESSING_DISABLED, "Work item processing is currently disabled");

‎src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
namespace Microsoft.OneFuzz.Service;
1313

1414
public interface IAdo {
15-
public Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Container container, IReport reportable, bool isLastRetryAttempt, Guid notificationId);
15+
public Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Container container, IReport reportable, Guid notificationId);
1616
}
1717

1818
public class Ado : NotificationsBase, IAdo {
@@ -24,7 +24,7 @@ public class Ado : NotificationsBase, IAdo {
2424
public Ado(ILogger<Ado> logTracer, IOnefuzzContext context) : base(logTracer, context) {
2525
}
2626

27-
public async Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Container container, IReport reportable, bool isLastRetryAttempt, Guid notificationId) {
27+
public async Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Container container, IReport reportable, Guid notificationId) {
2828
var filename = reportable.FileName();
2929
Report? report;
3030
if (reportable is RegressionReport regressionReport) {
@@ -57,22 +57,29 @@ public async Async.Task<OneFuzzResultVoid> NotifyAdo(AdoTemplate config, Contain
5757
_logTracer.LogEvent(adoEventType);
5858

5959
try {
60-
await ProcessNotification(_context, container, filename, config, report, _logTracer, notificationInfo, isRegression: reportable is RegressionReport);
60+
await ProcessNotification(_context, container, filename, config, report, _logTracer, notificationInfo,
61+
isRegression: reportable is RegressionReport);
6162
} catch (Exception e)
62-
when (e is VssUnauthorizedException || e is VssAuthenticationException || e is VssServiceException) {
63+
when (e is VssUnauthorizedException || e is VssAuthenticationException || e is VssServiceException) {
6364
if (config.AdoFields.TryGetValue("System.AssignedTo", out var assignedTo)) {
6465
_logTracer.AddTag("assigned_to", assignedTo);
6566
}
6667

67-
if (!isLastRetryAttempt && IsTransient(e)) {
68-
_logTracer.LogError("transient ADO notification failure {JobId} {TaskId} {Container} {Filename}", report.JobId, report.TaskId, container, filename);
69-
throw;
68+
if (IsTransient(e)) {
69+
_logTracer.LogError("transient ADO notification failure {JobId} {TaskId} {Container} {Filename}",
70+
report.JobId, report.TaskId, container, filename);
71+
72+
return OneFuzzResultVoid.Error(ErrorCode.TRANSIENT_NOTIFICATION_FAILURE,
73+
$"Failed to process ado notification : exception: {e}");
7074
} else {
7175
_logTracer.LogError(e, "Failed to process ado notification");
7276
await LogFailedNotification(report, e, notificationId);
7377
return OneFuzzResultVoid.Error(ErrorCode.NOTIFICATION_FAILURE,
7478
$"Failed to process ado notification : exception: {e}");
7579
}
80+
} catch (Exception e) {
81+
return OneFuzzResultVoid.Error(ErrorCode.NOTIFICATION_FAILURE,
82+
$"Failed to process ado notification : exception: {e}");
7683
}
7784
return OneFuzzResultVoid.Ok;
7885
}

‎src/pytypes/onefuzztypes/enums.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ class ErrorCode(Enum):
306306
ADO_VALIDATION_INVALID_PROJECT = 496
307307
INVALID_RETENTION_PERIOD = 497
308308
INVALID_CLI_VERSION = 498
309+
TRANSIENT_NOTIFICATION_FAILURE = 499
309310
# NB: if you update this enum, also update Enums.cs
310311

311312

0 commit comments

Comments
 (0)