Skip to content

Commit

Permalink
Add fine grained exceptions for known commit error situations
Browse files Browse the repository at this point in the history
To allow better error messages in UI.

Note: most of these shouldn't actually be possible in replace or combine APIs,
because the staging frames would run into these constraints
already when being created in Hastus import.

In addition there are lots of plain DB  constraints,
from simple value checks to foreign key constraints etc,
but I don't see it as beneficial to add custom exceptions for all of these.
Can always add these if users actually encounter them.
  • Loading branch information
Leitsi committed Nov 16, 2023
1 parent b334519 commit 78b8365
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,25 @@ data class TransactionSystemExtensions(
companion object {
fun from(ex: TransactionSystemException) = TransactionSystemExtensions(
HttpStatus.CONFLICT.value(),
HasuraErrorType.TransactionSystemError,
resolveHasuraErrorType(ex),
ex.cause?.message ?: "unknown error on transaction commit or rollback"
)

private fun resolveHasuraErrorType(ex: TransactionSystemException): HasuraErrorType {
val errorMessage = ex.cause?.message ?: "" // Should always be defined though.

val errorTypesWithMatchingSubstrings = listOf(
HasuraErrorType.PassingTimeStopPointMatchingOrderError to "passing times and their matching stop points must be in same order",
HasuraErrorType.PassingTimeFirstArrivalTimeError to "first passing time must not have arrival_time set",
HasuraErrorType.PassingTimeLastDepartureTimeError to "last passing time must not have departure_time set",
HasuraErrorType.PassingTimeNullError to "all passing time that are not first or last in the sequence must have both departure and arrival time defined",
HasuraErrorType.PassingTimesMixedJourneyPatternRefsError to "inconsistent journey_pattern_ref within vehicle journey, all timetabled_passing_times must reference same journey_pattern_ref as the vehicle_journey",
HasuraErrorType.ConflictingSchedulesError to "validate_queued_schedules_uniqueness",
HasuraErrorType.SequentialIntegrityError to "validate_service_sequential_integrity"
)

return errorTypesWithMatchingSubstrings.find { it.second in errorMessage }?.first
?: HasuraErrorType.TransactionSystemError
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@ package fi.hsl.jore4.timetables.api.util

enum class HasuraErrorType {
UnknownError,
ConflictingSchedulesError,
InvalidTargetPriorityError,
MultipleTargetFramesFoundError,
PassingTimeFirstArrivalTimeError,
PassingTimeLastDepartureTimeError,
PassingTimeNullError,
PassingTimeStopPointMatchingOrderError,
PassingTimesMixedJourneyPatternRefsError,
SequentialIntegrityError,
StagingVehicleScheduleFrameNotFoundError,
TargetPriorityParsingError,
TargetVehicleScheduleFrameNotFoundError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class TimetablesCombineApiTest(@Autowired val mockMvc: MockMvc) {
"message": "JDBC Commit Failed",
"extensions": {
"code": 409,
"type": "TransactionSystemError",
"type": "ConflictingSchedulesError",
"sqlErrorMessage": "$sqlErrorMessage"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class TimetablesReplaceApiTest(@Autowired val mockMvc: MockMvc) {
"message": "JDBC Commit Failed",
"extensions": {
"code": 409,
"type": "TransactionSystemError",
"type": "ConflictingSchedulesError",
"sqlErrorMessage": "$sqlErrorMessage"
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package fi.hsl.jore4.timetables.api.util

import org.junit.jupiter.api.Test
import org.springframework.transaction.TransactionSystemException
import kotlin.test.assertEquals

class TransactionSystemExtensionsTest {

private fun createTransactionSystemExceptionWithCause(message: String): TransactionSystemException {
return TransactionSystemException("test exception", Exception(message))
}

@Test
fun `resolves PassingTimeStopPointMatchingOrderError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: passing times and their matching stop points must be in same order
Where: PL/pgSQL function passing_times.validate_passing_time_sequences()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.PassingTimeStopPointMatchingOrderError)
}

@Test
fun `resolves PassingTimeFirstArrivalTimeError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: first passing time must not have arrival_time set
Where: PL/pgSQL function passing_times.validate_passing_time_sequences()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.PassingTimeFirstArrivalTimeError)
}

@Test
fun `resolves PassingTimeLastDepartureTimeError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: last passing time must not have departure_time set
Where: PL/pgSQL function passing_times.validate_passing_time_sequences()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.PassingTimeLastDepartureTimeError)
}

@Test
fun `resolves PassingTimeNullError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: all passing time that are not first or last in the sequence must have both departure and arrival time defined
Where: PL/pgSQL function passing_times.validate_passing_time_sequences()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.PassingTimeNullError)
}

@Test
fun `resolves PassingTimesMixedJourneyPatternRefsError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: inconsistent journey_pattern_ref within vehicle journey, all timetabled_passing_times must reference same journey_pattern_ref as the vehicle_journey
Where: PL/pgSQL function passing_times.validate_passing_time_sequences()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.PassingTimesMixedJourneyPatternRefsError)
}

@Test
fun `resolves ConflictingSchedulesError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: conflicting schedules detected: vehicle schedule frame
Where: PL/pgSQL function vehicle_schedule.validate_queued_schedules_uniqueness()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.ConflictingSchedulesError)
}

@Test
fun `resolves SequentialIntegrityError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: Sequential integrity issues detected:
Where: PL/pgSQL function vehicle_service.validate_service_sequential_integrity()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.SequentialIntegrityError)
}

@Test
fun `resolves an unknown error as TransactionSystemError`() {
val exception = TransactionSystemExtensions.from(
createTransactionSystemExceptionWithCause(
"""
ERROR: something else:
Where: PL/pgSQL function somewhere_else()
""".trimIndent()
)
)
assertEquals(exception.type, HasuraErrorType.TransactionSystemError)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@ import fi.hsl.jore4.timetables.extensions.deepClone
import fi.hsl.jore4.timetables.extensions.getNested
import fi.hsl.jore4.timetables.repository.VehicleScheduleFrameRepository
import fi.hsl.jore4.timetables.repository.VehicleServiceRepository
import mu.KotlinLogging
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.transaction.TransactionSystemException
import java.util.UUID
import kotlin.test.assertContains
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNotNull
import kotlin.test.assertNull

private val LOGGER = KotlinLogging.logger {}

@IntTest
class CombineTimetablesServiceTest @Autowired constructor(
val combineTimetablesService: CombineTimetablesService,
Expand Down Expand Up @@ -332,6 +336,31 @@ class CombineTimetablesServiceTest @Autowired constructor(
assertEquals(listOf(UUID.fromString("bb2abc90-8e91-4b0b-a7e4-751a04e81ba3")), result)
assertNull(vehicleScheduleFrameRepository.fetchOneByVehicleScheduleFrameId(stagingFrameId))
}

@Test
fun `fails when combininga would result in invalid data`() {
val testData = TimetablesDataset.createFromResource("datasets/combine_multiple_targets.json")
testData.getNested("_vehicle_schedule_frames.target2")["validity_start"] = "2022-07-15"
testData.getNested("_vehicle_schedule_frames.target2")["validity_end"] = "2023-05-15"

timetablesDataInserterRunner.truncateAndInsertDataset(testData.toJSONString())

val stagingFrameId = UUID.fromString("e8d07c0d-575f-4cbe-bddb-ead5b2943638")
val exception = assertFailsWith<TransactionSystemException> {
combineTimetablesService.combineTimetables(
listOf(stagingFrameId),
TimetablesPriority.STANDARD
)
}

// Check that the error messages are somewhat in the format we expect. TransactionSystemExtensions depends on these.
assertEquals(exception.message, "JDBC commit failed")
val causeMessage = exception?.cause?.message
assertNotNull(causeMessage)
assertContains(causeMessage, "ERROR: conflicting schedules detected: vehicle schedule frame")
assertContains(causeMessage, "Where: PL/pgSQL function vehicle_schedule.validate_queued_schedules_uniqueness()")
assertContains(causeMessage, "SQL statement \"SELECT vehicle_schedule.validate_queued_schedules_uniqueness()")
}
}

@Nested
Expand Down

0 comments on commit 78b8365

Please sign in to comment.