necouchman commented on code in PR #1136:
URL: https://github.com/apache/guacamole-client/pull/1136#discussion_r2615184895
##########
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java:
##########
@@ -95,15 +95,13 @@ private GuacamoleRadiusChallenge
getRadiusChallenge(RadiusPacket challengePacket
RadiusAttribute stateAttr =
challengePacket.findAttribute(Attr_State.TYPE);
if (stateAttr == null) {
logger.error("Something went wrong, state attribute not present.");
- logger.debug("State Attribute turned up null, which shouldn't
happen in AccessChallenge.");
return null;
}
// We need to get the reply message so we know what to ask the user
RadiusAttribute replyAttr =
challengePacket.findAttribute(Attr_ReplyMessage.TYPE);
if (replyAttr == null) {
logger.error("No reply message received from the server.");
- logger.debug("Expecting a Attr_ReplyMessage attribute on this
packet, and did not get one.");
Review Comment:
Similar to the above change - the `logger.error()` version of this message
might need to get some of the details from the `debug()` message that's being
removed.
##########
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java:
##########
@@ -95,15 +95,13 @@ private GuacamoleRadiusChallenge
getRadiusChallenge(RadiusPacket challengePacket
RadiusAttribute stateAttr =
challengePacket.findAttribute(Attr_State.TYPE);
if (stateAttr == null) {
logger.error("Something went wrong, state attribute not present.");
- logger.debug("State Attribute turned up null, which shouldn't
happen in AccessChallenge.");
Review Comment:
I wonder if it's worth a tweak to the error message just above this to be a
little more clear on when this error is occurring? Something like:
```
logger.error("Missing \"state\" attribute during the access challenge
step.");
```
I clearly wrote the messages above (and just below this) in pairs, expecting
that, if one happened and you turned on debugging, it'd be obvious what was
going on. Since the debug-level messages are being removed, here, the error
messages may need a little more verbosity?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
