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

Webhook Delays #801

Closed
mattisking opened this issue Sep 2, 2022 · 13 comments
Closed

Webhook Delays #801

mattisking opened this issue Sep 2, 2022 · 13 comments
Assignees
Labels

Comments

@mattisking
Copy link
Contributor

The current mocking set I’m working on requires lots of callbacks rather than traditional responses, but I need to be able to set delays on each callback and they may differ. We can set delays on the major response but not on the webhooks.

here’s the functionality in Wiremock.org:
https://wiremock.org/docs/webhooks-and-callbacks/

Additionally, we’d like the webhook responses to be fire and forget. They shouldn’t wait on a response.

@mattisking
Copy link
Contributor Author

ok, I've built in the most basic support for this locally. Will take more time to look at the code to figure out the random delay bits. Also, "fire and forget" works as well but I admit it's not pretty at the moment.

@StefH
Copy link
Collaborator

StefH commented Sep 5, 2022

Ah.
I missed your question here...
Sorry.

I'll also take a look for

  • fire and forget
  • setting delay

@StefH StefH self-assigned this Sep 5, 2022
@mattisking
Copy link
Contributor Author

mattisking commented Sep 5, 2022

One of the reasons for the need for fire and forget is that let’s say I define 3 callbacks (webhooks). Currently the code as it is today won’t return from the call until all webhooks have completed…. But webhooks are supposed to be totally asynchronous. The call in shouldn’t be blocked at all by waiting on what should be disconnected webhooks.

The service I’m mocking right now returns immediately with a 200. It then expects a webhook “Ack” immediately and then a Webhook some 30 seconds to 2 minutes later with the actual response. The webhooks support falls short on both. I’ll share what I added to the code today. I expect the Tasks handling can be done cleaner. Also, I coded a static delay but really want a random delay between two amounts.

@StefH
Copy link
Collaborator

StefH commented Sep 5, 2022

Fire and Forget : see preview version 1.5.5-ci-16422

@StefH
Copy link
Collaborator

StefH commented Sep 5, 2022

Do you need the delay before or after the webhook is called?

@mattisking
Copy link
Contributor Author

mattisking commented Sep 5, 2022

Before.

I had something like this in mind:
``
var webhook1 = new Webhook
{
Request = new WebhookRequest
{
Url = "http://localhost:12345",
Method = "post",
BodyData = new BodyData
{
BodyAsString = "1",
DetectedBodyType = BodyType.String,
DetectedBodyTypeFromContentType = BodyType.String
},
RequestProcessingDelay = TimeSpan.FromSeconds(10),
FireAndForget = true
}
};

        var webhook2 = new Webhook
        {
            Request = new WebhookRequest
            {
                Url = "http://localhost:12345",
                Method = "post",
                BodyData = new BodyData
                {
                    BodyAsString = "2",
                    DetectedBodyType = BodyType.String,
                    DetectedBodyTypeFromContentType = BodyType.String
                },
                RequestProcessingDelay = TimeSpan.FromSeconds(20),
                FireAndForget = false
            }
        };

        server
            .Given(Request
                .Create()
                .WithPath(new WildcardMatcher("/test*", true))
                .UsingGet())
            .WithWebhook(webhook1, webhook2)
            .RespondWith(Response.Create()
                .WithHeader("Content-Type", "application/json")
                .WithSuccess()
                .WithBody(@"{ ""result"": ""Ok""}"));
                ``

and in WebhookSender:
79:
``
// Create HttpRequestMessage
var httpRequestMessage = HttpRequestMessageHelper.Create(requestMessage, request.Url);

    var delay = request.RequestProcessingDelay;

    if (delay != null)
    {
        await Task.Delay(delay.Value);
    }

    // Call the URL
    return await client.SendAsync(httpRequestMessage);

``

Here's the trick, though... In order to make the delays non-additive, the fire and forget needs to be used and all the webhooks should get blasted off about the same time, each with it's own delay. To accomplish that, I had something like this:

WireMockMiddleware:

private async Task SendToWebhooksAsync(IMapping mapping, IRequestMessage request, IResponseMessage response)
        {
            var tasks = new List<Func<Task>>();

            if (mapping.Webhooks != null && mapping.Webhooks.Length > 0)
            {
                foreach (var webhook in mapping.Webhooks)
                {
                    var httpClientForWebhook = HttpClientBuilder.Build(mapping.Settings.WebhookSettings ?? new WebhookSettings());
                    var webhookSender = new WebhookSender(mapping.Settings);

                    tasks.Add(async () =>
                    {
                        try
                        {
                            await webhookSender.SendAsync(httpClientForWebhook, mapping, webhook.Request, request, response);
                        }
                        catch (Exception ex)
                        {
                            _options.Logger.Error($"Sending message to Webhook [] from Mapping '{mapping.Guid}' failed. Exception: {ex}");
                        }
                    });
                }
            }

            // Here you can use the await if you want to wait or skip the await or however you want skip the waiting
            //await Task.WhenAll(tasks.Select(async task => await task.Invoke()));
            await Task.WhenAll(tasks.Select(async task => task.Invoke()));
        }

@mattisking
Copy link
Contributor Author

Not gotten the hang of this editor. Above I'm spinning out all the webhooks at about the same time as tasks I can await (or not). This allows each of the Delays to be independent of each other.

@StefH
Copy link
Collaborator

StefH commented Sep 5, 2022

@mattisking
I used a different approach.
See PR:
#803

And comment/review the PR.

A preview version will be available in some time (just take the latest)

@mattisking
Copy link
Contributor Author

The goal here is to wait x amount before firing each webhook. The actual mocked API request should return immediately (unless a global delay is used or a configured delay on the mapping), regardless of the webhooks. Then the webhooks should fire after the configured delay for each webhook, independent of each other.

Example:
Let's say I'm calling a service that will take a significant time, perhaps, to come to a decision. I send the API request and the API immediately returns a 200.

The API I'm connecting to (and mocking) then sends it's information to another process that kicks off an "Acknowledgement" webhook that my system is waiting on. This happens within a few seconds.

Then the API I've sent the request to sends a "Decision" webhook maybe 30 seconds later that my system is waiting on.

The initial request should return immediately without waiting on the webhooks to run since webhooks are meant to be completely separate from the request. Using a flag for that is fine with me (UseFireAndForget) just in case there's some other use cases that need the response from the request after the webhooks have fired off and been consumed. Then the first webhook should come in a couple seconds later. Then the second webhook maybe 30 seconds after, and both webhooks should fire independent of each other. For instance, if I configure 5 seconds on webhook 1 and 5 seconds on webhook 2, webhooks 1 and 2 should both fire after 5 seconds and not 1 in 5 seconds and the next 5 seconds after that (which would make it 10 seconds).

That's why you see the approach I took.

@StefH
Copy link
Collaborator

StefH commented Sep 6, 2022

@mattisking
I think I understand.

I've modified the code according to your proposal. Please see the updated PR.

@mattisking
Copy link
Contributor Author

I've branched off yours with my suggested change. Basically it boils down to:

  1. Remove the bit from WebhookSender.cs at line 105 and the added function FireAndForget.
  2. Add to WebhookRequest/IWebhookRequest: public bool? UseFireAndForget { get; set; }
  3. WireMockMiddleware: line 226:
            await FireWebhooks(tasks, mapping.UseWebhooksFireAndForget ?? false);
        }

        private async Task FireWebhooks(List<Func<Task>> sendTasks, bool fireAndForget = false)
        {
            if (fireAndForget == true)
            {
                try
                {
                    // Do not await
                    await Task.WhenAll(sendTasks.Select(async task => task.Invoke()));
                }
                catch
                {
                    // Ignore
                }
            }
            else
            {
                await Task.WhenAll(sendTasks.Select(async task => await task.Invoke()));
            }
        }
  1. IMapping/Mapping/MappingModel:
    public bool? UseWebhooksFireAndForget { get; set; }
    (Add to Mapping constructor)

  2. IRespondWithAProvider:
    ///


    /// Support FireAndForget for any configured Webhooks
    ///

    ///
    ///
    IRespondWithAProvider WithWebhookFireAndForget(bool UseWebhooksFireAndForget);

  3. RespondWithAProvider:
    private bool _useWebhookFireAndForget = false;
    ...
    public IRespondWithAProvider WithWebhookFireAndForget(bool useWebhooksFireAndForget)
    {
    _useWebhookFireAndForget = useWebhooksFireAndForget;

     return this;
    

    }

  4. ProxyHelper: (add to Mapping constructor call)

  5. Update unit test constructors of Mapping.

@mattisking
Copy link
Contributor Author

I created a PR but I targeted your existing branch. If you approve of it, merge it into your own stef-webhooks branch and then you can merge yours. Or I can redo the PR against master. Wasn't sure so just based mine on your work.

@StefH
Copy link
Collaborator

StefH commented Sep 12, 2022

New NuGet will be released soon.

@StefH StefH closed this as completed Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants