Distinguish between retrialable and non-retriable errors when publishing messages with pubsub

[artursouza]

In what area(s)?

/area runtime

/area operator

/area placement

/area docs

/area test-and-release

What version of Dapr?

1.8.x

Expected Behavior

Should log WARN instead.

Actual Behavior

Logs ERROR because it returns an error to the upstream call stack:

dapr/pkg/runtime/runtime.go

Line 1937 in 490ef74

return errors.Errorf("RETRY status returned from app while processing pub/sub event %v", cloudEvent[pubsub.IDField])

dapr/pkg/runtime/runtime.go

Line 1945 in 490ef74

return errors.Errorf("unknown status returned from app while processing pub/sub event %v: %v", cloudEvent[pubsub.IDField], appResponse.Status)

dapr/pkg/runtime/runtime.go

Line 1960 in 490ef74

return errors.Errorf("retriable error returned from app while processing pub/sub event %v, topic: %v, body: %s. status code returned: %v", cloudEvent[pubsub.IDField], cloudEvent[pubsub.TopicField], body, statusCode)

Code should handle retriable vs non-retriable error as different return objects

Steps to Reproduce the Problem

Release Note

RELEASE NOTE:

[yaron2]

Maybe this should be Debug logs?

[SpikeWong]

@artursouza I want to make sure I understand correctly that for retriable errors we don't need to log Error, but just log Warn or log Debug, while for non-retriable errors we need to log Error.
The log behavior is done by upstream call, so we need different types of error to distinguish retriable from non-retriable error, is that right?

[artursouza]

Maybe this should be Debug logs?

I think it should log warning since users have asked for this behavior before.

[artursouza]

@artursouza I want to make sure I understand correctly that for retriable errors we don't need to log Error, but just log Warn or log Debug, while for non-retriable errors we need to log Error.

The log behavior is done by upstream call, so we need different types of error to distinguish retriable from non-retriable error, is that right?

Correct. It probably means that the function should return two error objects, one for retriable error and one for non-retriable.

[SpikeWong]

/assign

[SpikeWong]

@artursouza The act of catching these errors and logging them happens in the pubsub component(e.g. rabbitmq) of the components-contrib repo, right?
So if I want to change the log level I also need to change the components-contrib repository?

[artursouza]

@artursouza The act of catching these errors and logging them happens in the pubsub component(e.g. rabbitmq) of the components-contrib repo, right? So if I want to change the log level I also need to change the components-contrib repository?

Not necessarily. This code is handled here:

dapr/pkg/runtime/runtime.go

Line 761 in 490ef74

err = policy(func(ctx context.Context) error {

I think we can move the log logic there as well and we will have two error objects to differentiate between non-retriable and retriable errors. The change on each component to understand the difference can be a follow up.