Skip to content

Commit 432800b

Browse files
committed
Better choices for stackTraceKey
Found a few spots where `stackTraceKey` was including id hashes, resulting in separate crash reporter error groups per process. Fixed that and over-documented the rules. Glad this isn't public API.
1 parent 54e0b46 commit 432800b

File tree

3 files changed

+22
-4
lines changed

3 files changed

+22
-4
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ internal class RealRenderContext<out PropsT, StateT, OutputT>(
7272
key: String,
7373
handler: (ChildOutputT) -> WorkflowAction<PropsT, StateT, OutputT>
7474
): ChildRenderingT {
75-
checkNotFrozen(child) { "renderChild(${child.identifier})" }
75+
checkNotFrozen(child.identifier) {
76+
"renderChild(${child.identifier})"
77+
}
7678
return renderer.render(child, props, key, handler)
7779
}
7880

@@ -111,6 +113,7 @@ internal class RealRenderContext<out PropsT, StateT, OutputT>(
111113

112114
/**
113115
* @param stackTraceKey ensures unique crash reporter error groups.
116+
* It is important that keys are stable across processes, avoid system hashes.
114117
*
115118
* @see checkWithKey
116119
*/

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,10 @@ internal class SubtreeManager<PropsT, StateT, OutputT>(
126126
// Prevent duplicate workflows with the same key.
127127
workflowTracer.trace("CheckingUniqueMatches") {
128128
children.forEachStaging {
129-
requireWithKey(!(it.matches(child, key, workflowTracer)), stackTraceKey = child) {
130-
"Expected keys to be unique for ${child.identifier}: key=\"$key\""
131-
}
129+
requireWithKey(
130+
!(it.matches(child, key, workflowTracer)),
131+
stackTraceKey = child.identifier
132+
) { "Expected keys to be unique for ${child.identifier}: key=\"$key\"" }
132133
}
133134
}
134135

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.squareup.workflow1.internal
22

3+
import com.squareup.workflow1.Workflow
4+
import com.squareup.workflow1.identifier
35
import kotlin.contracts.ExperimentalContracts
46
import kotlin.contracts.contract
57

@@ -10,6 +12,10 @@ import kotlin.contracts.contract
1012
*
1113
* So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
1214
*
15+
* @param stackTraceKey an object whose [toString] method will serve as a grouping key
16+
* for crash reporters. It is important that keys are stable across processes,
17+
* avoid system hashes.
18+
*
1319
* @see [withKey]
1420
*
1521
* @throws IllegalArgumentException if the [value] is false.
@@ -37,6 +43,10 @@ internal inline fun requireWithKey(
3743
*
3844
* So far [stackTraceKey] is only effective on JVM, it has no effect in other languages.
3945
*
46+
* @param stackTraceKey an object whose [toString] method will serve as a grouping key
47+
* for crash reporters. It is important that keys are stable across processes,
48+
* avoid system hashes.
49+
*
4050
* @see [withKey]
4151
*
4252
* @throws IllegalStateException if the [value] is false.
@@ -62,5 +72,9 @@ internal inline fun checkWithKey(
6272
* that crash reporter's default grouping will create unique groups for unique keys.
6373
*
6474
* So far only effective on JVM, this is a pass through in other languages.
75+
*
76+
* @param stackTraceKey an object whose [toString] method will serve as a grouping key
77+
* for crash reporters. It is important that keys are stable across processes,
78+
* avoid system hashes.
6579
*/
6680
internal expect fun <T : Throwable> T.withKey(stackTraceKey: Any): T

0 commit comments

Comments
 (0)