Evict the workflow from cache if their is a panic in the SDK #1654
Conversation
internal/internal_task_pollers.go
Outdated
| tagPanicError, fmt.Sprintf("%v", p), | ||
| tagPanicStack, st) | ||
| taskErr = newPanicError(p, st) | ||
| err = taskErr |
There was a problem hiding this comment.
doesn't this refer to the err on line 382?
There was a problem hiding this comment.
err refers to the named return . Otherwise the test would fail.
There was a problem hiding this comment.
oh right, 382 reuses and doesn't shadow err.. I kind of hate that feature of go :( separate names would make it more clear
There was a problem hiding this comment.
Yeah good point, idk why I didn't give it a different name.
|
Pardon my ignorance here, but how is eviction performed on task failure today unrelated to this panic specifically? Is it the next time the task is received the cached workflow is determined invalid? |
We rely on |
|
Still a bit lost (sorry, trying to avoid code). So today eviction occurs when on panic? And after this PR when will it happen? Or are you saying eviction was not occurring on this panic and the cached value was reused even post-panic? Meaning your test here would have failed? |
Yes, eviction was not occurring on panics in the SDK code. Panics in users workflows are translated to errors that are being evicted. |
Quinn-With-Two-Ns
force-pushed
the
evict-on-internal-panic
branch
from
cdb4ff6
to
24dd149
Compare
Evict the workflow from cache if their is a panic in the SDK. previously if their was some panic in the SDK code the SDK would not evict the workflow from cache and leave the workflow in a potentially bad state