Skip to content
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

Fix panic on webhooks without messages #1156

Merged
merged 2 commits into from
Jan 25, 2023

Conversation

nguyer
Copy link
Contributor

@nguyer nguyer commented Jan 24, 2023

Resolves #1155

This PR changes the behavior of webhook callbacks in several ways:

  • If reply is specified on the subscription it no longer panics if there is no Message on the event (such as is the case with custom contract events)
  • If withData is specified subscription, but no Message is on the event, the event payload itself will still be sent

This means that subscriptions can be setup with either the reply and withData fields set (or both) and still use the same subscription for custom contract events as well.

Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
@nguyer nguyer merged commit 15d5c76 into hyperledger:main Jan 25, 2023
@nguyer nguyer deleted the webhooks-fix branch January 25, 2023 18:58
default:
// Otherwise just send the first object directly
case len(allData) == 1:
// Just send the first object directly
req.r.SetBody(firstData)
Copy link
Contributor

Choose a reason for hiding this comment

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

In retrospect, Codecov is saying that this line is now missing coverage...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants