sendgrid-python icon indicating copy to clipboard operation
sendgrid-python copied to clipboard

HTTP requests to SendGrid can hang as SendGridAPIClient does not have a timeout.

Open matheusbrat opened this issue 4 years ago • 4 comments

I'm creating a new issue as the other one was closed but the issue reported on: https://github.com/sendgrid/sendgrid-python/issues/757 still happens using sendgrid=6.6.0 and Python 3.7.

Issue Summary

HTTP requests to SendGrid could hang, as SendGridAPIClient has no time out defined in case of a network anomaly.

Steps to Reproduce

This is a bit difficult to reproduce, as it could be caused by a network anomaly (Not necessarily SendGrid's issue).

When a network anomaly occurred, it is possible for API calls to SendGrid to hang indefinitely.

  • By default, the HTTP client's timeout defaults to None.
  • When SendGridAPIClient creates a client, it does not allow a timeout to be passed in.
  • User of the client, unfortunately, has no way to provide a timeout, unless s/he creates a wrapper on top of SendGridAPIClient to override SendGridAPIClient.client or to have an outer wrapper to control timeout.

Expected Behavior Either a reasonable timeout is set, or a timeout option should be allowed when creating a SendGridAPIClient.

Actual None is available.

Technical details:

  • sendgrid-python Version: master
  • Python Version: 2.7
  • python-http-client Version: master

Relevant Link:

  • How HTTP Client is created in SendGridAPIClient: https://github.com/sendgrid/sendgrid-python/blob/265b9848834f3a2ff2c3d92fb1b711cd7f33f438/sendgrid/sendgrid.py#L75
  • python-http-client: https://github.com/sendgrid/python-http-client/blob/master/python_http_client/client.py#L71

matheusbrat avatar Apr 13 '21 22:04 matheusbrat

@x-warrior Thanks for reopening the issue. This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

shwetha-manvinkurke avatar Apr 14 '21 23:04 shwetha-manvinkurke

@shwetha-manvinkurke Tossing our hat in the ring for this. In the past week, the Sendgrid backend has been hanging much worse and has now required engineering efforts around the python API. We are left to either roll our own, or wait for this.

Something that impacts deliverability I would hope is prioritized internally by product teams.

qeternity avatar May 19 '21 13:05 qeternity

I recently had the same issue.

I wanted to send out emails to hundreds of customers, but the SendGrid API would hang for certain periods of time. As a result, many of the emails that should have been sent were not sent, and we had no choice but to retry manually.

It is more important reduce frequency of hanging API (as root cause improvement), but It would be better to have another option for error handling on the client side.

hassaku63 avatar Apr 18 '22 09:04 hassaku63

Although a naive implementation, I think that adding a timeout with default value=None to the Client class constructor would solve this issue.

However, I tried to implement this idea but could not figure out how to write integration tests.

To intentionally delay the response, the mocking server (sendgrid/sendgrid-oai) configuration would need to be changed. It is not possible to test the timeout behavior in the case of a delayed response with this repository alone.

At this time, I have no concrete implementation ideas on how sendgrid-oai should be modified. I only have a diff with no test code to this repository. Am I allowed to send a pull request in this situation? Should I request changes to sendgrid-oai first?

(There is a CLI option in prism called "--callback-delay", and it seems possible to simulate response delays by specifying this option when starting the mocking server. If this is correct, I think we need to add new support for launching in delayed mode to sendgrid-oai's mocking server.)

hassaku63 avatar May 03 '22 01:05 hassaku63