Skip to content

Commit

Permalink
fix(webhooks): fix skipping webhooks calls on timeouts (#2501)
Browse files Browse the repository at this point in the history
  • Loading branch information
khvn26 authored Jul 21, 2023
1 parent 915b67a commit 583e248
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
62 changes: 62 additions & 0 deletions api/webhooks/tests/test_webhooks.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import hashlib
import hmac
import json
from typing import Type
from unittest import TestCase, mock

import pytest
from core.constants import FLAGSMITH_SIGNATURE_HEADER
from pytest_mock import MockerFixture
from requests.exceptions import ConnectionError, Timeout

from environments.models import Environment, Webhook
from organisations.models import Organisation, OrganisationWebhook
Expand Down Expand Up @@ -149,3 +152,62 @@ def test_request_does_not_have_signature_header_if_secret_is_not_set(
# Then
_, kwargs = mock_requests.post.call_args_list[0]
assert FLAGSMITH_SIGNATURE_HEADER not in kwargs["headers"]


@pytest.mark.parametrize("expected_error", [ConnectionError, Timeout])
def test_call_environment_webhooks__multiple_webhooks__failure__calls_expected(
mocker: MockerFixture,
expected_error: Type[Exception],
environment: Environment,
) -> None:
# Given
requests_post_mock = mocker.patch("webhooks.webhooks.requests.post")
requests_post_mock.side_effect = expected_error()
send_failure_email_mock: mock.Mock = mocker.patch(
"webhooks.webhooks.send_failure_email"
)

webhook_1 = Webhook.objects.create(
url="http://url.1.com",
enabled=True,
environment=environment,
)
webhook_2 = Webhook.objects.create(
url="http://url.2.com",
enabled=True,
environment=environment,
)

expected_data = {}
expected_event_type = WebhookEventType.FLAG_UPDATED
expected_send_failure_email_data = {
"event_type": expected_event_type.value,
"data": expected_data,
}
expected_send_failure_status_code = f"N/A ({expected_error.__name__})"

# When
call_environment_webhooks(
environment=environment,
data=expected_data,
event_type=expected_event_type,
)

# Then
assert send_failure_email_mock.call_count == 2
send_failure_email_mock.assert_has_calls(
[
mocker.call(
webhook_2,
expected_send_failure_email_data,
WebhookType.ENVIRONMENT,
expected_send_failure_status_code,
),
mocker.call(
webhook_1,
expected_send_failure_email_data,
WebhookType.ENVIRONMENT,
expected_send_failure_status_code,
),
]
)
9 changes: 7 additions & 2 deletions api/webhooks/webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,13 @@ def _call_webhook_email_on_error(
):
try:
res = _call_webhook(webhook, data)
except requests.exceptions.ConnectionError:
send_failure_email(webhook, data, webhook_type)
except requests.exceptions.RequestException as exc:
send_failure_email(
webhook,
data,
webhook_type,
f"N/A ({exc.__class__.__name__})",
)
return

if res.status_code != 200:
Expand Down

3 comments on commit 583e248

@vercel
Copy link

@vercel vercel bot commented on 583e248 Jul 21, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

docs – ./docs

docs-flagsmith.vercel.app
docs.bullet-train.io
docs-git-main-flagsmith.vercel.app
docs.flagsmith.com

@vercel
Copy link

@vercel vercel bot commented on 583e248 Jul 21, 2023

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on 583e248 Jul 21, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.