camunda / zeebe-process-test

Testing project for Camunda Platform 8
40 stars 15 forks source link

1011 null pointer when sending out response with no request metadata #1087

Closed ana-vinogradova-camunda closed 6 months ago

ana-vinogradova-camunda commented 6 months ago

Description

This PR is related to this bug. This was fixed and backported to 8.4 and 8.3 this week.

The initial idea was to fix ZPT GrpcResponseWriter tryWriteResponse method to contain similar check that was introduced in Zeebe project:

    if (command.hasRequestMetadata()) {
      responseWriter.writeEventOnCommand(eventKey, SignalIntent.BROADCASTED, signalRecord, command);
    }

More details are available in this comment here. I am wondering if this is still valid as after the fix for this bug the response for broadcast signal events that have no corresponding request is not sent anymore:

This check in SignalBroadcastProcessor:

    if (command.hasRequestMetadata()) {
      responseWriter.writeEventOnCommand(eventKey, SignalIntent.BROADCASTED, signalRecord, command);
    }

is happening before GrpcResponseWriter is called.

I wonder if an extra check should still be added to GrpcResponseWriter? Do we have other scenarios when we don't have request metadata available? I will add the check if so πŸ‘

What is in this draft PR:

Related issues

closes #1011

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

Testing:

Documentation:

github-actions[bot] commented 6 months ago

Test Results

 50 files  Β±0   50 suites  Β±0   2m 25s :stopwatch: +34s 136 tests +1  136 :white_check_mark: +1  0 :zzz: Β±0  0 :x: Β±0  432 runsβ€Š +3  432 :white_check_mark: +3  0 :zzz: Β±0  0 :x: Β±0 

Results for commit ea3bbb74. ± Comparison against base commit 66657e54.

ana-vinogradova-camunda commented 6 months ago

@tmetzke thank you for the suggestion πŸ‘

I think that it is a good idea to fix this on the response writer-level as well while working on this bug. I am not sure where this issue might happen, but I'll investigate.

I will address other comments as well πŸ‘

ana-vinogradova-camunda commented 6 months ago

Closing as decided on different implementation.