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

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

Open
matheusbrat opened this issue Apr 13, 2021 · 4 comments
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap

Comments

@matheusbrat
Copy link

matheusbrat commented Apr 13, 2021

I'm creating a new issue as the other one was closed but the issue reported on: #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:

@shwetha-manvinkurke shwetha-manvinkurke added status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap labels Apr 14, 2021
@shwetha-manvinkurke
Copy link
Contributor

@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.

@qeternity
Copy link

@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.

@hassaku63
Copy link

hassaku63 commented Apr 18, 2022

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
Copy link

hassaku63 commented May 3, 2022

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

4 participants