tempest icon indicating copy to clipboard operation
tempest copied to clipboard

Query hangs for missing attributes

Open amitdev opened this issue 2 years ago • 2 comments

This is probably an edge case, but if a record does not have a matching attribute in the data class for AsyncInlineView, the query just "hangs". I don't know if there is a timeout that kicks in after some time, but it doesn't return at least for a 30 seconds.

This happens if we change the "schema" of a table. For eg, if we have something like this:

data class PlaylistInfo(
  @Attribute(name = "partition_key")
  val playlist_token: String,
  val playlist_name: String,
  val playlist_tracks: List<AlbumTrack.Key>
)

and later add a new attribute:

data class PlaylistInfo(
  @Attribute(name = "partition_key")
  val playlist_token: String,
  val playlist_name: String,
  val playlist_tracks: List<AlbumTrack.Key>,
  val playlist_version: Long
)

If we try to read the old records using the new format it hangs (even if we make playlist_version nullable and provide a default value).

It should throw an exception in these cases instead of hanging. Also it would be nice to be able to read old records if the new attributes are nullable.

amitdev avatar Jun 27 '22 09:06 amitdev

The problems seems to be that if https://github.com/cashapp/tempest/blob/c0fe0a9fbfdcb08e1edd1dc62ade4e847d4842cb/tempest2/src/main/kotlin/app/cash/tempest2/internal/DynamoDbQueryable.kt#L113 throws an exception it just seems to get stuck.

amitdev avatar Jun 27 '22 09:06 amitdev

This is because the SdkPublisher.map call will only catch exceptions inheriting from RuntimeException. See this line in the AWS SDK. When a data class in Kotlin can't be initialized, it throws exceptions inheriting from ReflectiveOperationException.

data class TestClass(val nonNullableMap: Map<String, String>)
val testClassConstructor = TestClass::class.constructors.first()
val someArgs = mapOf(testClassConstructor.parameters.first() to null)

try {
  testClassConstructor.callBy(someArgs)
} catch (exc: ReflectiveOperationException) {
  println("This gets called")
} catch (exc: RuntimeException) {
  println("This does not get called")
}

This means that Tempest gets stuck waiting forever for .map to come back with a result, which it never will, because the underlying coroutine has already terminated exceptionally.

polyatail avatar Jun 15 '23 00:06 polyatail