diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e8baa260..14d0cda42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to ## [Unreleased] - Update framework-api to 4.2.0 - Add table in system database 'event_error_log' for storing errors with events that failed to process +- New SystemErrorService for reporting system errors ## [6.4.0] - 2019-11-12 ### Changed diff --git a/framework-system/framework-system-errors/pom.xml b/framework-system/framework-system-errors/pom.xml new file mode 100644 index 000000000..951f74574 --- /dev/null +++ b/framework-system/framework-system-errors/pom.xml @@ -0,0 +1,53 @@ + + + + framework-system + uk.gov.justice.services + 6.5.0-SNAPSHOT + + 4.0.0 + + framework-system-errors + + + + javax + javaee-api + provided + + + uk.gov.justice.services + framework-system-persistence + ${project.version} + + + uk.gov.justice.framework-api + framework-api-system-errors + ${framework-api.version} + + + uk.gov.justice.framework-api + framework-api-core + + + uk.gov.justice.services + framework-utilities + ${project.version} + + + + + junit + junit + test + + + org.mockito + mockito-core + test + + + + diff --git a/framework-system/framework-system-errors/src/main/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorService.java b/framework-system/framework-system-errors/src/main/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorService.java new file mode 100644 index 000000000..451fe6e3b --- /dev/null +++ b/framework-system/framework-system-errors/src/main/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorService.java @@ -0,0 +1,71 @@ +package uk.gov.justice.services.framework.system.errors; + +import static java.lang.String.format; +import static javax.transaction.Transactional.TxType.NOT_SUPPORTED; + +import uk.gov.justice.services.common.util.UtcClock; +import uk.gov.justice.services.framework.system.errors.SystemErrorService; +import uk.gov.justice.services.framework.utilities.exceptions.StackTraceProvider; +import uk.gov.justice.services.messaging.JsonEnvelope; +import uk.gov.justice.services.messaging.Metadata; +import uk.gov.justice.services.system.domain.EventError; +import uk.gov.justice.services.system.persistence.EventErrorLogRepository; + +import java.time.Clock; +import java.util.Optional; +import java.util.UUID; + +import javax.inject.Inject; +import javax.transaction.Transactional; + +public class DefaultSystemErrorService implements SystemErrorService { + + private static final Long MISSING_EVENT_NUMBER = -1L; + private static final String NO_COMMENT = ""; + + @Inject + private EventErrorLogRepository eventErrorLogRepository; + + @Inject + private StackTraceProvider stackTraceProvider; + + @Inject + private UtcClock clock; + + @Override + @Transactional(NOT_SUPPORTED) + public void reportError( + final String messageId, + final String componentName, + final JsonEnvelope jsonEnvelope, + final Throwable exception) { + + final Metadata metadata = jsonEnvelope.metadata(); + final UUID eventId = metadata.id(); + final Optional eventNumber = metadata.eventNumber(); + + final EventError eventError = new EventError( + eventId, + eventNumber.orElse(MISSING_EVENT_NUMBER), + componentName, + messageId, + metadata.asJsonObject().toString(), + jsonEnvelope.payload().toString(), + exception.getMessage(), + stackTraceProvider.getStackTrace(exception), + clock.now(), + getComment(eventNumber) + ); + + eventErrorLogRepository.save(eventError); + } + + private String getComment(final Optional eventNumberOptional) { + + if (eventNumberOptional.isPresent()) { + return NO_COMMENT; + } + + return format("Event number is missing from event. Setting to %d instead", MISSING_EVENT_NUMBER); + } +} diff --git a/framework-system/framework-system-errors/src/test/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorServiceTest.java b/framework-system/framework-system-errors/src/test/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorServiceTest.java new file mode 100644 index 000000000..3b4cc45c6 --- /dev/null +++ b/framework-system/framework-system-errors/src/test/java/uk/gov/justice/services/framework/system/errors/DefaultSystemErrorServiceTest.java @@ -0,0 +1,162 @@ +package uk.gov.justice.services.framework.system.errors; + +import static java.util.Optional.empty; +import static java.util.Optional.of; +import static java.util.UUID.randomUUID; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import uk.gov.justice.services.common.util.UtcClock; +import uk.gov.justice.services.framework.utilities.exceptions.StackTraceProvider; +import uk.gov.justice.services.messaging.JsonEnvelope; +import uk.gov.justice.services.messaging.Metadata; +import uk.gov.justice.services.system.domain.EventError; +import uk.gov.justice.services.system.persistence.EventErrorLogRepository; + +import java.time.ZonedDateTime; +import java.util.UUID; + +import javax.json.JsonObject; +import javax.json.JsonValue; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class DefaultSystemErrorServiceTest { + + private static final String AN_EMPTY_STRING = ""; + + @Mock + private EventErrorLogRepository eventErrorLogRepository; + + @Mock + private StackTraceProvider stackTraceProvider; + + @Mock + private UtcClock clock; + + @InjectMocks + private DefaultSystemErrorService defaultSystemErrorService; + + @Captor + private ArgumentCaptor eventErrorCaptor; + + @Test + public void shouldCreateEventErrorAndPersist() throws Exception { + + final String messageId = "message id"; + final String componentName = "EVENT_LISTENER"; + final String errorMessage = "Help help we're all going to die"; + final Throwable exception = new NullPointerException(errorMessage); + final Long eventNumber = 239874L; + final UUID eventId = randomUUID(); + final String metadataJson = "{metadata: stuff}"; + final String payloadJson = "{payload: stuff}"; + final String stackTrace = "the stacktrace"; + final ZonedDateTime erroredAt = new UtcClock().now(); + + final JsonEnvelope jsonEnvelope = mock(JsonEnvelope.class); + final Metadata metadata = mock(Metadata.class); + final JsonObject metadataJsonObject = mock(JsonObject.class); + final JsonValue payload = mock(JsonValue.class); + + when(jsonEnvelope.metadata()).thenReturn(metadata); + when(metadata.id()).thenReturn(eventId); + when(metadata.eventNumber()).thenReturn(of(eventNumber)); + + when(metadata.asJsonObject()).thenReturn(metadataJsonObject); + when(jsonEnvelope.payload()).thenReturn(payload); + + when(metadataJsonObject.toString()).thenReturn(metadataJson); + when(payload.toString()).thenReturn(payloadJson); + when(stackTraceProvider.getStackTrace(exception)).thenReturn(stackTrace); + + when(clock.now()).thenReturn(erroredAt); + + defaultSystemErrorService.reportError( + messageId, + componentName, + jsonEnvelope, + exception + ); + + verify(eventErrorLogRepository).save(eventErrorCaptor.capture()); + + final EventError eventError = eventErrorCaptor.getValue(); + + assertThat(eventError.getMessageId(), is(messageId)); + assertThat(eventError.getComponent(), is(componentName)); + assertThat(eventError.getErrorMessage(), is(errorMessage)); + assertThat(eventError.getEventNumber(), is(eventNumber)); + assertThat(eventError.getEventId(), is(eventId)); + assertThat(eventError.getMetadata(), is(metadataJson)); + assertThat(eventError.getPayload(), is(payloadJson)); + assertThat(eventError.getStacktrace(), is(stackTrace)); + assertThat(eventError.getErroredAt(), is(erroredAt)); + assertThat(eventError.getComments(), is(AN_EMPTY_STRING)); + } + + @Test + public void shouldHandleMissingEventNumber() throws Exception { + + final String messageId = "message id"; + final String componentName = "EVENT_LISTENER"; + final String errorMessage = "Help help we're all going to die"; + final Throwable exception = new NullPointerException(errorMessage); + final UUID eventId = randomUUID(); + final String metadataJson = "{metadata: stuff}"; + final String payloadJson = "{payload: stuff}"; + final String stackTrace = "the stacktrace"; + final ZonedDateTime erroredAt = new UtcClock().now(); + + final JsonEnvelope jsonEnvelope = mock(JsonEnvelope.class); + final Metadata metadata = mock(Metadata.class); + final JsonObject metadataJsonObject = mock(JsonObject.class); + final JsonValue payload = mock(JsonValue.class); + + when(jsonEnvelope.metadata()).thenReturn(metadata); + when(metadata.id()).thenReturn(eventId); + when(metadata.eventNumber()).thenReturn(empty()); + + when(metadata.asJsonObject()).thenReturn(metadataJsonObject); + when(jsonEnvelope.payload()).thenReturn(payload); + + when(metadataJsonObject.toString()).thenReturn(metadataJson); + when(payload.toString()).thenReturn(payloadJson); + when(stackTraceProvider.getStackTrace(exception)).thenReturn(stackTrace); + + when(clock.now()).thenReturn(erroredAt); + + defaultSystemErrorService.reportError( + messageId, + componentName, + jsonEnvelope, + exception + ); + + verify(eventErrorLogRepository).save(eventErrorCaptor.capture()); + + final EventError eventError = eventErrorCaptor.getValue(); + + assertThat(eventError.getMessageId(), is(messageId)); + assertThat(eventError.getComponent(), is(componentName)); + assertThat(eventError.getErrorMessage(), is(errorMessage)); + assertThat(eventError.getEventNumber(), is(-1L)); + assertThat(eventError.getEventId(), is(eventId)); + assertThat(eventError.getMetadata(), is(metadataJson)); + assertThat(eventError.getPayload(), is(payloadJson)); + assertThat(eventError.getStacktrace(), is(stackTrace)); + assertThat(eventError.getErroredAt(), is(erroredAt)); + assertThat(eventError.getComments(), is("Event number is missing from event. Setting to -1 instead")); + } +} diff --git a/framework-system/framework-system-liquibase/src/main/resources/liquibase/framework-system-changesets/005-add-comment-column-to-event-error-log-table.xml b/framework-system/framework-system-liquibase/src/main/resources/liquibase/framework-system-changesets/005-add-comment-column-to-event-error-log-table.xml new file mode 100644 index 000000000..71a3fd74c --- /dev/null +++ b/framework-system/framework-system-liquibase/src/main/resources/liquibase/framework-system-changesets/005-add-comment-column-to-event-error-log-table.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + diff --git a/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java b/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java index 04dd0f9ab..bd508632a 100644 --- a/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java +++ b/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/domain/EventError.java @@ -15,6 +15,7 @@ public class EventError { private final String errorMessage; private final String stacktrace; private final ZonedDateTime erroredAt; + private final String comments; public EventError( final UUID eventId, @@ -25,7 +26,8 @@ public EventError( final String payload, final String errorMessage, final String stacktrace, - final ZonedDateTime erroredAt) { + final ZonedDateTime erroredAt, + final String comments) { this.eventId = eventId; this.eventNumber = eventNumber; this.component = component; @@ -35,6 +37,7 @@ public EventError( this.errorMessage = errorMessage; this.stacktrace = stacktrace; this.erroredAt = erroredAt; + this.comments = comments; } public UUID getEventId() { @@ -73,6 +76,10 @@ public ZonedDateTime getErroredAt() { return erroredAt; } + public String getComments() { + return comments; + } + @Override public boolean equals(final Object o) { if (this == o) return true; @@ -86,12 +93,13 @@ public boolean equals(final Object o) { Objects.equals(payload, that.payload) && Objects.equals(errorMessage, that.errorMessage) && Objects.equals(stacktrace, that.stacktrace) && - Objects.equals(erroredAt, that.erroredAt); + Objects.equals(erroredAt, that.erroredAt) && + Objects.equals(comments, that.comments); } @Override public int hashCode() { - return Objects.hash(eventId, eventNumber, component, messageId, metadata, payload, errorMessage, stacktrace, erroredAt); + return Objects.hash(eventId, eventNumber, component, messageId, metadata, payload, errorMessage, stacktrace, erroredAt, comments); } @Override @@ -106,6 +114,7 @@ public String toString() { ", errorMessage='" + errorMessage + '\'' + ", stacktrace='" + stacktrace + '\'' + ", erroredAt=" + erroredAt + + ", comments='" + comments + '\'' + '}'; } } diff --git a/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/persistence/EventErrorLogRepository.java b/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/persistence/EventErrorLogRepository.java index fb6b9b45d..60ffc6d84 100644 --- a/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/persistence/EventErrorLogRepository.java +++ b/framework-system/framework-system-persistence/src/main/java/uk/gov/justice/services/system/persistence/EventErrorLogRepository.java @@ -1,5 +1,6 @@ package uk.gov.justice.services.system.persistence; +import static javax.transaction.Transactional.TxType.REQUIRES_NEW; import static uk.gov.justice.services.common.converter.ZonedDateTimes.fromSqlTimestamp; import static uk.gov.justice.services.common.converter.ZonedDateTimes.toSqlTimestamp; @@ -17,15 +18,16 @@ import javax.inject.Inject; import javax.sql.DataSource; +import javax.transaction.Transactional; public class EventErrorLogRepository { private static final String INSERT_EVENT_ERROR_LOG_SQL = "INSERT INTO event_error_log " + - "(event_id, event_number, component, message_id, metadata, payload, error_message, stacktrace, errored_at) " + - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"; + "(event_id, event_number, component, message_id, metadata, payload, error_message, stacktrace, errored_at, comments) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"; private static final String FIND_ALL_EVENT_ERROR_LOG_SQL = "SELECT " + - "event_id, event_number, component, message_id, metadata, payload, error_message, stacktrace, errored_at " + + "event_id, event_number, component, message_id, metadata, payload, error_message, stacktrace, errored_at, comments " + "FROM event_error_log"; private static final String TRUNCATE_EVENT_ERROR_LOG_SQL = "TRUNCATE event_error_log CASCADE"; @@ -33,6 +35,7 @@ public class EventErrorLogRepository { @Inject private SystemJdbcDataSourceProvider systemJdbcDataSourceProvider; + @Transactional(REQUIRES_NEW) public void save(final EventError eventError) { final DataSource dataSource = systemJdbcDataSourceProvider.getDataSource(); @@ -49,6 +52,7 @@ public void save(final EventError eventError) { preparedStatement.setString(7, eventError.getErrorMessage()); preparedStatement.setString(8, eventError.getStacktrace()); preparedStatement.setTimestamp(9, toSqlTimestamp(eventError.getErroredAt())); + preparedStatement.setString(10, eventError.getComments()); preparedStatement.executeUpdate(); } catch (final SQLException e) { throw new SystemPersistenceException("Failed to insert into event_error_log", e); @@ -74,6 +78,7 @@ public List findAll() { final String errorMessage = resultSet.getString("error_message"); final String stacktrace = resultSet.getString("stacktrace"); final ZonedDateTime erroredAt = fromSqlTimestamp(resultSet.getTimestamp("errored_at")); + final String comment = resultSet.getString("comments"); final EventError eventError = new EventError( eventId, @@ -84,7 +89,8 @@ public List findAll() { payload, errorMessage, stacktrace, - erroredAt + erroredAt, + comment ); eventErrors.add(eventError); diff --git a/framework-system/framework-system-persistence/src/test/java/uk/gov/justice/services/system/persistence/EventErrorLogRepositoryIT.java b/framework-system/framework-system-persistence/src/test/java/uk/gov/justice/services/system/persistence/EventErrorLogRepositoryIT.java index 7d9ff715d..b2da4261f 100644 --- a/framework-system/framework-system-persistence/src/test/java/uk/gov/justice/services/system/persistence/EventErrorLogRepositoryIT.java +++ b/framework-system/framework-system-persistence/src/test/java/uk/gov/justice/services/system/persistence/EventErrorLogRepositoryIT.java @@ -1,5 +1,7 @@ package uk.gov.justice.services.system.persistence; +import static java.util.Optional.empty; +import static java.util.Optional.of; import static java.util.UUID.randomUUID; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; @@ -12,6 +14,7 @@ import java.time.ZonedDateTime; import java.util.List; +import java.util.Optional; import javax.sql.DataSource; @@ -50,7 +53,8 @@ public void shouldStoreEventErrorLog() throws Exception { "payload_1", "errorMessage_1", "stacktrace_1", - now + now, + "comment_1" ); final EventError eventError_2 = new EventError( randomUUID(), @@ -61,7 +65,8 @@ public void shouldStoreEventErrorLog() throws Exception { "payload_2", "errorMessage_2", "stacktrace_2", - then + then, + "comment_2" ); eventErrorLogRepository.save(eventError_1); diff --git a/framework-system/pom.xml b/framework-system/pom.xml index f793d6fe9..cc8a5fd48 100644 --- a/framework-system/pom.xml +++ b/framework-system/pom.xml @@ -16,5 +16,6 @@ framework-management framework-system-persistence framework-system-liquibase + framework-system-errors