Add payloadData and expiration to the TokenExpiredException exception #63
Conversation
|
Thanks for this change! I left few minor comments. |
| { | ||
| get | ||
| { | ||
| if (Data.Contains(PayloadDataKey)) |
There was a problem hiding this comment.
Can you move this snippet to a helper method? To reuse, also make both getters one-liners. Also do we really need to check a key to exist? I'd let it throw.
There was a problem hiding this comment.
I was debating whether to throw or not... The problem I have with throwing is that since you requested that TokenExpiredException inherits from SignationVerificationException (see PR #33), the Received and Expected properties will not be assigned for TokenExpiredException... so making a catch (SignationVerificationException ex) may crash if we don't do that...
There was a problem hiding this comment.
Sure, let's keep it safe. Can you please move it a helper? And remove a space inside the cast operator, to follow the common style.
| @@ -264,7 +264,12 @@ public static void Verify(string payloadJson, string decodedCrypto, string decod | |||
| var secondsSinceEpoch = Math.Round((DateTime.UtcNow - UnixEpoch).TotalSeconds); | |||
| if (secondsSinceEpoch >= expInt) | |||
| { | |||
| throw new TokenExpiredException("Token has expired."); | |||
| var tokenExpiredException = new TokenExpiredException("Token has expired.") | |||
There was a problem hiding this comment.
You can throw right away, without a variable assignment.
There was a problem hiding this comment.
Oh, true, unfinished refactoring ;)
There was a problem hiding this comment.
Cool! Can you please format the document (usually Ctrl+K,D) to make sure no extra/missing spaces were added/removed?
There was a problem hiding this comment.
I'll try. I'll need to change my settings, cause Ctrl-K,D will result in tabs all over otherwise! 😛
There was a problem hiding this comment.
Yeah, I feel you pain, I'm a tabs guy too! Don't get me started 😉 Usually what I do is format then find and replace \t with (2 or 4 spaces).
| { | ||
| get | ||
| { | ||
| if (Data.Contains(PayloadDataKey)) |
There was a problem hiding this comment.
Sure, let's keep it safe. Can you please move it a helper? And remove a space inside the cast operator, to follow the common style.
Adding more contextual information to the
TokenExpiredExceptionfor usage by the parent application