Skip to content

Commit 53af996

Browse files
committed
- Change the RockQueue's Drain() queue to write unhandled exceptions to the Rock Log instead of polluting the ExceptionLog and retry register controllers transaction 5 times before giving up.
1 parent 1df71d3 commit 53af996

File tree

6 files changed

+114
-35
lines changed

6 files changed

+114
-35
lines changed

Rock.Rest/Swagger/SwaggerV2CacheBuilder.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@
2222
using System.Web.Http;
2323
using System.Web.Http.Routing;
2424

25+
using Microsoft.Extensions.Logging;
26+
27+
using Rock.Logging;
2528
using Rock.Model;
2629
using Rock.Utility;
2730

31+
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Rest.Swagger" )]
2832
namespace Rock.Rest.Swagger
2933
{
3034
/// <summary>
@@ -76,7 +80,8 @@ public void OnStartup()
7680
}
7781
catch ( Exception ex )
7882
{
79-
ExceptionLogService.LogException( new Exception( "Failed to initialize the API documentation during startup.", ex ) );
83+
var logger = RockLogger.LoggerFactory.CreateLogger( "Rock.Rest.Swagger" );
84+
logger.LogWarning( ex, "Failed to initialize the API documentation during startup." );
8085
}
8186
} );
8287
}

Rock/Model/CMS/RestController/RestControllerService.WebForms.cs

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using System.Diagnostics;
2020
using System.Linq;
2121
using System.Reflection;
22+
using System.Threading;
2223
using System.Web.Http;
2324
using System.Web.Http.Controllers;
2425

@@ -59,29 +60,71 @@ public override string ToString()
5960
}
6061

6162
/// <summary>
62-
/// Registers the controllers.
63+
/// Registers the controllers; retrying up to 5 times if an InvalidOperationException
64+
/// (Collection was modified; enumeration operation may not execute) occurs.
6365
/// </summary>
6466
public static void RegisterControllers()
6567
{
6668
/*
67-
* 05/13/2022 MDP/DMV
68-
*
69-
* In addition to the 12/19/2019 BJW note, we also added a RockGuid attribute to
70-
* controllers and methods (except for inherited methods). This will prevent
71-
* loosing security on methods that have changed their signature.
72-
*
73-
*
74-
* 12/19/2019 BJW
75-
*
76-
* There was an issue with the SecuredAttribute not calculating API ID the same as was being calculated here.
77-
* This caused the secured attribute to sometimes not find the RestAction record and thus not find the
78-
* appropriate permissions (Auth table). The new method "GetApiId" is used in both places as a standardized
79-
* API ID generator to ensure that this does not occur. The following code has also been modified to gracefully
80-
* update any old style API IDs and update them to the new format without losing any foreign key associations, such
81-
* as permissions.
82-
*
83-
* See task for detailed background: https://app.asana.com/0/474497188512037/1150703513867003/f
84-
*/
69+
6/2/2025 - NA
70+
71+
IApiExplorer.ApiDescriptions returns a Collection<ApiDescription> that can be lazily populated
72+
by ApiExplorer at access time. If other threads are also accessing or modifying routes/controllers
73+
during this time (e.g., app initialization, concurrent requests), we can hit this concurrency
74+
exception even when just reading the .Count property.
75+
76+
Reason: Try 5 times before giving up.
77+
*/
78+
79+
int maxAttempts = 5;
80+
int baseDelayMs = 3000; // 3 seconds base
81+
82+
for ( int attempt = 1; attempt <= maxAttempts ; attempt++ )
83+
{
84+
try
85+
{
86+
RegisterControllersInternal();
87+
break; // success, exit loop
88+
}
89+
catch ( InvalidOperationException )
90+
{
91+
if ( attempt == maxAttempts )
92+
{
93+
throw; // fail after final attempt
94+
}
95+
96+
// small delay
97+
Thread.Sleep( baseDelayMs * attempt );
98+
}
99+
}
100+
}
101+
102+
/// <summary>
103+
/// Registers the controllers.
104+
/// </summary>
105+
private static void RegisterControllersInternal()
106+
{
107+
/*
108+
5/13/2022 - MDP/DMV
109+
110+
In addition to the 12/19/2019 BJW note, we also added a RockGuid attribute to
111+
controllers and methods (except for inherited methods). This will prevent
112+
loosing security on methods that have changed their signature.
113+
114+
115+
12/19/2019 - BJW
116+
117+
There was an issue with the SecuredAttribute not calculating API ID the same as was being calculated here.
118+
This caused the secured attribute to sometimes not find the RestAction record and thus not find the
119+
appropriate permissions (Auth table). The new method "GetApiId" is used in both places as a standardized
120+
API ID generator to ensure that this does not occur. The following code has also been modified to gracefully
121+
update any old style API IDs and update them to the new format without losing any foreign key associations, such
122+
as permissions.
123+
124+
See task for detailed background: https://app.asana.com/0/474497188512037/1150703513867003/f
125+
126+
Reason: Preserve security settings when method signatures change and ensure consistent API ID generation.
127+
*/
85128

86129
// Controller Class Name => New Format Id => Old Format Id
87130
var controllerApiIdMap = new Dictionary<string, Dictionary<string, string>>();

Rock/Properties/AssemblyInfo.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@
5454
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Financial" )]
5555
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Lava" )]
5656
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Model" )]
57+
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Transactions" )]
58+
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Transactions.RockQueue" )]
5759
[assembly: Rock.Logging.RockLoggingCategory( "Rock.Web" )]
5860
[assembly: Rock.Logging.RockLoggingCategory( "RockWeb.Blocks" )]
5961
[assembly: Rock.Logging.RockLoggingCategory( "RockWeb.App_Code.TwilioDefaultResponseAsync" )]

Rock/Transactions/RegisterControllersTransaction.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@
2020
namespace Rock.Transactions
2121
{
2222
/*
23-
05-02-2022 MDP
24-
We changed this back to use Rock.Transactions.RegisterControllersTransaction instead.
25-
This is because RestControllerService.RegisterControllers has a dependancy on having
26-
all the Routes configured. The Bus message approach (Rock.Tasks.RegisterRestControllers)
27-
would sometimes result in RegisterControllers happening before Routes would be configured,
28-
which would cause RestController and RestAction records to get deleted.
23+
5/2/2022 - MDP
2924
30-
*/
25+
We changed this back to use Rock.Transactions.RegisterControllersTransaction instead.
26+
This is because RestControllerService.RegisterControllers has a dependency on having
27+
all the Routes configured. The Bus message approach (Rock.Tasks.RegisterRestControllers)
28+
would sometimes result in RegisterControllers happening before Routes would be configured,
29+
which would cause RestController and RestAction records to get deleted.
30+
31+
Reason: RestControllerService.RegisterControllers requires Routes to be configured first.
32+
*/
3133

3234
/// <summary>
3335
/// Registers controllers

Rock/Transactions/RockQueue.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public static void Drain( Action<Exception> errorHandler )
239239
}
240240
catch ( Exception ex )
241241
{
242-
errorHandler( new Exception( $"Exception in RockQueue.Drain(): {transaction.GetType().Name}", ex ) );
242+
errorHandler( new Exception( $"Unhandled exception in RockQueue.Drain(): {transaction.GetType().Name} {transaction.ToJson()}", ex ) );
243243
}
244244
}
245245
}

RockWeb/App_Code/Global.asax.cs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@
2727
using System.Web.Http;
2828
using System.Web.Optimization;
2929
using System.Web.Routing;
30-
30+
using Microsoft.Extensions.Logging;
3131
using Rock;
3232
using Rock.Blocks;
3333
using Rock.Communication;
3434
using Rock.Configuration;
3535
using Rock.Data;
3636
using Rock.Enums.Cms;
37+
using Rock.Logging;
3738
using Rock.Model;
3839
using Rock.Observability;
3940
using Rock.Security;
@@ -43,6 +44,7 @@
4344
using Rock.Web.UI;
4445
using Rock.WebStartup;
4546

47+
[assembly: Rock.Logging.RockLoggingCategory( "RockWeb.Global" )]
4648
namespace RockWeb
4749
{
4850
/// <summary>
@@ -235,7 +237,7 @@ private void ApplicationStartupStage2()
235237

236238
SetError66();
237239
var startupException = new RockStartupException( "Error occurred during application startup", ex );
238-
LogError( startupException, null );
240+
LogException( startupException, null );
239241
throw startupException;
240242
}
241243

@@ -358,7 +360,7 @@ private static void StartEnsureChromeEngineThread()
358360
}
359361
catch ( Exception ex )
360362
{
361-
LogError( ex, null );
363+
LogException( ex, null );
362364
}
363365
} ).Start();
364366
}
@@ -1194,17 +1196,42 @@ public static void DrainTransactionQueue()
11941196
if ( !Global.QueueInUse )
11951197
{
11961198
Global.QueueInUse = true;
1197-
RockQueue.Drain( ( ex ) => LogError( ex, null ) );
1199+
RockQueue.Drain( ( ex ) => WriteErrorToRockLog( ex, "Rock.Transactions", null ) );
11981200
Global.QueueInUse = false;
11991201
}
12001202
}
12011203

1204+
/// <summary>
1205+
/// A handler for Rock Logging exception messages via Rock Logger.
1206+
/// If a message is provided, it will log that message otherwise
1207+
/// it will log the exception message.
1208+
/// </summary>
1209+
/// <param name="ex"></param>
1210+
private static void WriteErrorToRockLog( Exception ex, string loggerCategory, string message )
1211+
{
1212+
if ( string.IsNullOrWhiteSpace( loggerCategory ) )
1213+
{
1214+
loggerCategory = "RockWeb.Global";
1215+
}
1216+
1217+
var logger = RockLogger.LoggerFactory.CreateLogger( loggerCategory );
1218+
1219+
if ( !string.IsNullOrWhiteSpace( message ) )
1220+
{
1221+
logger.LogError( ex, message );
1222+
}
1223+
else
1224+
{
1225+
logger.LogError( ex.Message );
1226+
}
1227+
}
1228+
12021229
/// <summary>
12031230
/// Logs the error to database
12041231
/// </summary>
12051232
/// <param name="ex">The ex.</param>
12061233
/// <param name="context">The context.</param>
1207-
private static void LogError( Exception ex, HttpContext context )
1234+
private static void LogException( Exception ex, HttpContext context )
12081235
{
12091236
int? pageId;
12101237
int? siteId;
@@ -1278,7 +1305,7 @@ public static void CacheItemRemoved( string k, object v, CacheItemRemovedReason
12781305
}
12791306
catch ( Exception ex )
12801307
{
1281-
LogError( ex, null );
1308+
LogException( ex, null );
12821309
}
12831310
}
12841311

@@ -1308,7 +1335,7 @@ Rock.SystemKey.SystemSetting.ENABLE_KEEP_ALIVE can be enabled in Rock's System S
13081335
}
13091336
catch ( Exception ex )
13101337
{
1311-
LogError( new Exception( "Error doing KeepAlive request.", ex ), null );
1338+
LogException( new Exception( "Error doing KeepAlive request.", ex ), null );
13121339
}
13131340
}
13141341
}

0 commit comments

Comments
 (0)