Merge pull request #1310 from square/ray/no-id-hashes-in-group-keys

Better choices for `stackTraceKey`

Author: rjrjr

workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt

@@ -40,7 +40,8 @@ internal class WorkerWorkflow<OutputT>(
       unsnapshottableIdentifier(workerType)
     }
 
-  override fun describeRealIdentifier(): String = workerType.toString()
+  override fun describeRealIdentifier(): String =
+    workerType.toString().replace(" (Kotlin reflection is not available)", "")
 
   override fun initialState(
     props: Worker<OutputT>,

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/RealRenderContext.kt

@@ -72,7 +72,9 @@ internal class RealRenderContext<out PropsT, StateT, OutputT>(
     key: String,
     handler: (ChildOutputT) -> WorkflowAction<PropsT, StateT, OutputT>
   ): ChildRenderingT {
-    checkNotFrozen(child) { "renderChild(${child.identifier})" }
+    checkNotFrozen(child.identifier) {
+      "renderChild(${child.identifier})"
+    }
     return renderer.render(child, props, key, handler)
   }
 
@@ -111,6 +113,7 @@ internal class RealRenderContext<out PropsT, StateT, OutputT>(
 
   /**
    * @param stackTraceKey ensures unique crash reporter error groups.
+   * It is important that keys are stable across processes, avoid system hashes.
    *
    * @see checkWithKey
    */

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt

@@ -126,9 +126,10 @@ internal class SubtreeManager<PropsT, StateT, OutputT>(
     // Prevent duplicate workflows with the same key.
     workflowTracer.trace("CheckingUniqueMatches") {
       children.forEachStaging {
-        requireWithKey(!(it.matches(child, key, workflowTracer)), stackTraceKey = child) {
-          "Expected keys to be unique for ${child.identifier}: key=\"$key\""
-        }
+        requireWithKey(
+          !(it.matches(child, key, workflowTracer)),
+          stackTraceKey = child.identifier
+        ) { "Expected keys to be unique for ${child.identifier}: key=\"$key\"" }
       }
     }
 

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/Throwables.kt

@@ -10,6 +10,10 @@ import kotlin.contracts.contract
  *
  * So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
  *
+ * @param stackTraceKey an object whose [toString] method will serve as a grouping key
+ * for crash reporters. It is important that keys are stable across processes,
+ * avoid system hashes.
+ *
  * @see [withKey]
  *
  * @throws IllegalArgumentException if the [value] is false.
@@ -37,6 +41,10 @@ internal inline fun requireWithKey(
  *
  * So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
  *
+ * @param stackTraceKey an object whose [toString] method will serve as a grouping key
+ * for crash reporters. It is important that keys are stable across processes,
+ * avoid system hashes.
+ *
  * @see [withKey]
  *
  * @throws IllegalStateException if the [value] is false.
@@ -62,5 +70,9 @@ internal inline fun checkWithKey(
  * that crash reporter's default grouping will create unique groups for unique keys.
  *
  * So far only effective on JVM, this is a pass through in other languages.
+ *
+ * @param stackTraceKey an object whose [toString] method will serve as a grouping key
+ * for crash reporters. It is important that keys are stable across processes,
+ * avoid system hashes.
  */
 internal expect fun <T : Throwable> T.withKey(stackTraceKey: Any): T