-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Hello,
I was exploring your library and caught an exception during runtime. I had forgot to configure the TimeSpan on the RevokeToken `IserviceCollectionMethod.
/// <summary>
/// Register default InMemory BlackList Store Service using <seealso cref="MemoryCacheBlackList" />
/// </summary>
/// <param name="services">The services</param>
/// <param name="defaultTtl">The default ttl</param>
/// <returns>The services</returns>
public static IServiceCollection AddRevokeMemoryCacheStore(this IServiceCollection services, TimeSpan? defaultTtl = null)
{
services.TryAddSingleton<IBlackList>(provider => new MemoryCacheBlackList(provider.GetService<IMemoryCache>(), defaultTtl));
return services;
}You know the library better than I do, are there any 'run-time' use-case configurations where defaultTtl will be null and this method will be called?
I understand that null is allowed for this method, however, when Revoke is called while null is configured for defaultTtl an exception is thrown similar to:
"message": "System.ArgumentOutOfRangeException: The added or subtracted value results in an un-representable DateTime. (Parameter 'value')\r\n at System.DateTime.ThrowDateArithmetic(Int32 param)\r\n at System.DateTime.AddTicks(Int64 value)\r\n at System.DateTime.Add(TimeSpan value)\r\n at Revoke.NET.MemoryBlackList.Revoke(String key)"I would like to add a null check to ensure that an error is thrown on startup instead of runtime -- However, I am not sure if there is any reason why this will be marked as null during the startup.
Question
Is there any reason why a user will call this method below and be happy with a null value for TimeSpan?
// This or similar... note that the TimeSpan will be null
services.AddRevokeInMemoryStore().AddJWTBearerTokenRevokeMiddleware();Propose
-
Option 1: Do nothing, there is a reason why it will be null, ignore it.
-
Option 2: Add a null check that is thrown at runtime if the value is null.
-
Option 3: Remove 'allowed null's for this method
-
Option 4: Add a default value of XX Time in Days or Minutes
You know the library better than I do, what do you suggest?
Thanks