-
Notifications
You must be signed in to change notification settings - Fork 20
[CAPPL-536] Workflow update not picked up by Syncer #1029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3ba24f4
46388e4
17473fd
1987a00
10352b5
559bf4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -215,7 +215,24 @@ func (t *triggerExecutableServer) RegisterTrigger(request *capabilitiespb.Trigge | |
} | ||
responseCh, err := t.impl.RegisterTrigger(server.Context(), req) | ||
if err != nil { | ||
return fmt.Errorf("error registering trigger: %w", err) | ||
// the first message sent to the client will be an ack or error message, this is done in order to syncronize the client and server and avoid | ||
// errors to unregister not found triggers. If the error is not nil, we send an error message to the client and return the error | ||
msg := &capabilitiespb.TriggerResponseMessage{ | ||
Message: &capabilitiespb.TriggerResponseMessage_Response{ | ||
Response: &capabilitiespb.TriggerResponse{ | ||
Error: err.Error(), | ||
}, | ||
}, | ||
} | ||
return server.Send(msg) | ||
} | ||
|
||
// Send ACK response to client | ||
msg := &capabilitiespb.TriggerResponseMessage{ | ||
Message: &capabilitiespb.TriggerResponseMessage_Ack{}, | ||
} | ||
if err = server.Send(msg); err != nil { | ||
return fmt.Errorf("failed sending ACK response for trigger registration %s: %w", request, err) | ||
} | ||
|
||
defer func() { | ||
|
@@ -270,6 +287,17 @@ func (t *triggerExecutableClient) RegisterTrigger(ctx context.Context, req capab | |
return nil, fmt.Errorf("error registering trigger: %w", err) | ||
} | ||
|
||
// In order to ensure the registration is successful, we need to wait for the first message from the server. | ||
// This will be an ack or error message. If the error is not nil, we return an error. | ||
ackMsg, err := responseStream.Recv() | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to receive registering trigger ack message: %w", err) | ||
} | ||
|
||
if ackMsg.GetAck() == nil { | ||
return nil, errors.New(fmt.Sprintf("failed registering trigger: %s", ackMsg.GetResponse().GetError())) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @agparadiso This code is a bit ambiguous I think: what happens if the server sends a response without an ack? We should clearly error in that case as that would mean the protocol has been violated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed to send: |
||
|
||
return forwardTriggerResponsesToChannel(ctx, t.Logger, req, responseStream.Recv) | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.