Ian UI d2 1394 update attest failure reaction #125
Conversation
| @@ -33,12 +34,11 @@ public class AttestationTokenRetriever { | |||
| private final AtomicReference<String> attestationToken; | |||
| private final AtomicReference<String> optOutJwt; | |||
| private final AtomicReference<String> coreJwt; | |||
| private final Handler<Integer> responseWatcher; | |||
| private final Handler<Pair<Integer, String>> responseWatcher; | |||
There was a problem hiding this comment.
Are you sure this is only used by operator?
There was a problem hiding this comment.
Out of the core, optout, admin, and operator, the attestation token retriever is used by operator and optout, but optout uses a null responseWatcher. Will look into optout closer to make sure it is not broken by these changes
There was a problem hiding this comment.
I have tested optout/core with my shared changes locally and it is able to attest and refresh with no extra changes needed
src/main/java/com/uid2/shared/attest/AttestationTokenRetriever.java
Outdated
Show resolved
Hide resolved
src/main/java/com/uid2/shared/attest/AttestationTokenRetriever.java
Outdated
Show resolved
Hide resolved
src/test/java/com/uid2/shared/attest/AttestationTokenRetrieverTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/uid2/shared/attest/AttestationTokenRetrieverTest.java
Outdated
Show resolved
Hide resolved
# Conflicts: # src/main/java/com/uid2/shared/attest/AttestationTokenRetriever.java # src/test/java/com/uid2/shared/attest/AttestationTokenRetrieverTest.java
| @@ -112,6 +115,7 @@ private void attestationExpirationCheck(long timerId) { | |||
| } | |||
|
|
|||
| try { | |||
|
|
|||
src/main/java/com/uid2/shared/attest/AttestationTokenRetriever.java
Outdated
Show resolved
Hide resolved
| notifyResponseWatcher(401, e.getMessage()); | ||
| LOGGER.info("Re-attest failed: ", e); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Sorry to do this as I know you have been trying to get this approved for a while, but can we log out for both the 401 and the 500 the response code, and change the messages so we can tell what the cause was?
There was a problem hiding this comment.
I am not quite sure what you are suggesting. It will already log out for both a 401 and a 500, with the message being based on the exception that occurred. Are you suggesting we generalize the error that is logged, so that a 401 always logs the same thing and a 500 always logs the same thing? And if so what do you suggest for the message?
There was a problem hiding this comment.
What I want to make sure is that we can tell the difference in our logs (so we can alert differently) between the 2 types of 401 errors. If you are confident that this is the case, then I am OK with this.
src/test/java/com/uid2/shared/attest/AttestationTokenRetrieverTest.java
Outdated
Show resolved
Hide resolved
Update shared to inform attestation response watcher of both status code and response body.