Skip to content

Conversation

@Shaddoll
Copy link
Member

What changed?

  • recover from panics on replication task processing

Why?

  • to prevent bad data/legacy buggy code from causing crashes

How did you test it?
unit tests

Potential risks

Release notes

Documentation Changes

tag.WorkflowRunID(attr.GetRunID()),
tag.Value(r),
)
err = fmt.Errorf("handleActivityTask panic: %v", r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mind emitting a metric indicating a panic is captured, we don't want to miss this

func (p *taskProcessorImpl) processResponse(response *types.ReplicationMessages) {
defer func() {
if r := recover(); r != nil {
p.logger.Error("processResponse encountered panic.", tag.Value(r))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as before, if possible, can we emit a metric indicating a captured panic ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants