Send 504 response when Gambit requests timeout

[aaronschachter]

FEATURE OVERVIEW

User Story

As a Developer -- I want to know if a request is taking longer than 30 seconds and receive a 504 back.

Active routes to check:

• POST /chatbot
• POST /signups
• POST /campaigns/:id/message
• GET /campaigns/
• GET /campaigns/:id

Note: Not including DonorsChoose until keep is confirmed ( #857 )

Additional Information (optional)

https://devcenter.heroku.com/articles/request-timeout

When a timeout is detected the router will return a customizable error page to the client and an H12 error is emitted to your application logs. While the router has returned a response to the client, your application will not know that the request it is processing has reached a time-out, and your application will continue to work on the request. To avoid this situation Heroku recommends setting a timeout within your application and keeping the value well under 30 seconds, such as 10 or 15 seconds. Unlike the routing timeout, these timers will begin when the request begins being processed by your application. You can read more about this below in Timeout behavior.

Why This Matters

Blink will rely this response to retry a request if a 504 is sent back.

Current Workaround w/o Feature (if applicable)

N/A

[aaronschachter]

This seems promising: https://www.npmjs.com/package/connect-timeout

[aaronschachter]

Tried out of the new TODO's to use Promises to only return a Gambit 200 back to Blink upon getting a successful Mobile Commons POST /profile_updateresponse in this commit -- https://github.com/DoSomething/gambit/commit/d43c4d725363cdad85b58528c9ae2b8da05d80a9. Check out the timestamps for waiting for the MC response back:

4:26:40 PM web.1 |  debug: helpers.replacePhoenixCampaignVars campaignId:2900
4:26:40 PM web.1 |  verbose: saved user:5547be89469c64ec7d8b518d current_campaign:2900
4:26:40 PM web.1 |  debug: mobilecommons.executePost:profile_update
4:26:40 PM web.1 |  debug: stathat response:200 postStat:'gambit-dev - phoenix: GET signups 200'
4:27:09 PM web.1 |  debug: res.text
4:27:10 PM web.1 |  debug:  
4:27:10 PM web.1 |  { success: 'true' }
4:27:10 PM web.1 |  error: Request timed out after 15 seconds.
4:27:11 PM web.1 |  debug: stathat response:200 postStat:'gambit-dev - 504: Request timed out after 15 seconds.'

The 29 second response time of the POST /profile_update is not Promising (get it?). If we want Blink to confirm the message was delivered to the User's phone, our best bet sounds like Blink making the post to Mobile Commons to deliver the message once it gets the appropriate reply message from Gambit... instead of relying on Gambit to deliver the SMS.

Sample Gambit POST /chatbot response that Blink could use to make the post to Mobile Commons:

{
  "success": {
    "code": 200,
    "message": "Picking up where you left off on Bumble Bands...\n\nSend your best pic of you and the 33 bumble bands you created."
  }
}

cc @sergii-tkachenko

If we go this route, we could definitely close #762 as posting to Mobile Commons to deliver messages would be something that Gambit no longer needs to worry about....

[aaronschachter]

Documenting that Gambit doesn't (and still won't) return a 200 based on whether we get a successful POST profile_update from Mobile Commons -- we only return a 200 when we've processed the incoming request and determined the reply message to send. Blink will assume responsibility for ensuring the message gets delivered via Mobile Commons in #862 .

[aaronschachter]

The last 2 endpoints that don't check for timeouts yet are the ones used the most infrequently:

• GET /campaigns -- Used by Quicksilver to cache campaigns, Slothbot
• GET /campaigns/:id -- Used by Slothbot to return rendered messages

The GET /campaigns index endpoint is a little bit of a dumpster fire in the sense that it's making extra calls to Contentful it doesn't need to, for sake of writing less code (it's just calling its helper fetchCampaign for each Campaign, which queries out for keywords we could have saved in our first Contentful request to grab all Campaigns with keywords).

#863 checks for POST /campaign/:id/message but ideally we shouldn't close this issue until we add our check for req.timedout to the GET callbacks, which would require a bit of lower priority cleanup.

Adding the 504 checks to our 3 POST endpoints should put us in great shape for Blink to handle mData requests and forward them to Gambit though. cc @sergii-tkachenko @rapala61