Skip to content

Commit

Permalink
merge: #9934
Browse files Browse the repository at this point in the history
9934: Don't log JsonParseExceptions as errors r=remcowesterhoud a=remcowesterhoud

## Description
JsonParseExceptions get thrown when a user passes variables in their request which cannot be converted to message pack. Currently these are being logged as errors. Since this a fault by the user we should log these as debug.

<!-- Which issues are closed by this PR or are related -->

closes #9933



Co-authored-by: Remco Westerhoud <remco@westerhoud.nl>
  • Loading branch information
zeebe-bors-camunda[bot] and remcowesterhoud committed Aug 2, 2022
2 parents 4be7a04 + c6dfa7d commit 101750b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
package io.camunda.zeebe.gateway.grpc;

import com.fasterxml.jackson.core.JsonParseException;
import com.google.protobuf.Any;
import com.google.rpc.Code;
import com.google.rpc.Status;
Expand Down Expand Up @@ -72,6 +73,9 @@ private Status mapErrorToStatus(
builder.setCode(Code.INVALID_ARGUMENT_VALUE).setMessage(error.getMessage());
logger.debug(
"Expected to handle gRPC request, but messagepack property was invalid", rootError);
} else if (error instanceof JsonParseException) {
builder.setCode(Code.INVALID_ARGUMENT_VALUE).setMessage(error.getMessage());
logger.debug("Expected to handle gRPC request, but JSON property was invalid", rootError);
} else if (error instanceof PartitionNotFoundException) {
builder.setCode(Code.UNAVAILABLE_VALUE).setMessage(error.getMessage());
logger.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
package io.camunda.zeebe.gateway.grpc;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

import com.fasterxml.jackson.core.JsonParseException;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.rpc.Status;
import io.camunda.zeebe.gateway.cmd.BrokerErrorException;
import io.camunda.zeebe.gateway.impl.broker.RequestRetriesExhaustedException;
import io.camunda.zeebe.gateway.impl.broker.response.BrokerError;
import io.camunda.zeebe.protocol.impl.encoding.MsgPackConverter;
import io.camunda.zeebe.protocol.record.ErrorCode;
import io.camunda.zeebe.util.logging.RecordingAppender;
import io.grpc.Status.Code;
Expand Down Expand Up @@ -118,4 +121,26 @@ void shouldLogTimeoutExceptionWithCorrectStacktrace() {

assertThat(event.getThrown()).isEqualTo(executionException);
}

@Test
void shouldLogJsonParseExceptionOnDebug() {
// given
try {
MsgPackConverter.convertToMsgPack("{\"json\":\"invalid\"");
fail("Expected to throw exception");
} catch (final RuntimeException runtimeException) {
assertThat(runtimeException.getCause()).isInstanceOf(JsonParseException.class);
final JsonParseException exception = (JsonParseException) runtimeException.getCause();

// when
log.setLevel(Level.DEBUG);
final StatusRuntimeException statusException = errorMapper.mapError(exception, logger);

// then
assertThat(statusException.getStatus().getCode()).isEqualTo(Code.INVALID_ARGUMENT);
assertThat(recorder.getAppendedEvents()).hasSize(1);
final LogEvent event = recorder.getAppendedEvents().get(0);
assertThat(event.getLevel()).isEqualTo(Level.DEBUG);
}
}
}

0 comments on commit 101750b

Please sign in to comment.