I could be wrong here, but my understanding is that Twilio requires a valid TwiML response in order to consider a message processed. If that return is going back to Twilio (which I think it is), then this will make Twilio unhappy.

We should be able to get away with just <Response></Response> as a return, which is a valid return that does nothing.

Twilio didn't generate an error message when I tested it, contrary to their docs. (Perhaps because I'm sending a response with whatever Flask's default Content-Type is (text/plain maybe?) rather than the TwiML type, it's not expecting to be able to do anything with it?)

In any case, I agree that what you're suggesting is nicer, whether it's strictly required or not.

Oooh, this is cool! So every N minutes (n=4 in this case) we'll clear the SMS queue of messages? (So on average, we'd see a reply every two minutes?)

Yep! This seemed like the simplest way to have a delay before replying without actually making the Lambda function spin for a few minutes (and rack up charges in the process).

I thought about randomly skipping 50% of the queue every time send_sms runs to increase the variability, but I'm not sure whether that would make it more realistic or less.

Python n00b 'woah' moment here... You can have a try/except/else statement? That's cool!

Am I correct the else happens if no exception is raised?

Yep!

You can also attach an else block to a for or while loop, which executes if the loop runs to completion (as opposed to hitting a break).

Am I correct this expires our messages after a week, so if the same person were to text again after a week has passed the conversation would start again from the beginning?

Correct. I wasn't sure what the threshold should be, but I felt like there should be some cut-off.

Does this lose the message they sent us? It would be visible in the Twilio logs, but in terms of maximum amusement for line operators, being able to keep the conversation in a more accessible form would be amazing. :)

Or am I correct that these queued messages get deleted later (when we process them), so storing the SMS received in them is not exactly useful. :)

Would it make more sense to have a separate logs/ folder, which just logs incoming and outbound messages and timestamps,. but doesn't play a role in message queuing at all? (Absence of a logs directory isn't a blocker for this change, but is certainly a nice-to-have!)

Yes, that's exactly how it works! (I meant to add a PR comment summarising how everything works, but ran out of time.)

One of my least favourite things about JSON is that it doesn't allow comments... 😭

While Zappa may be relaxed enough to allow them, the QA/Testing part of my soul stays awake at night when I know we're technically breaking a format standard.

I know it's awful, but can I ask for something like:

"s3_bucket": "An S3 bucket that doesn't already exist. Zappa will create it and use it for storage when you run `zappa init`"

Comments are waaay nicer, and if we were using YAML or HJSON I'd be all over them. But every linter (including github's changes view) and my psyche is going to worry every time it sees a // otherwise. :/

Documentation! You are amazing! ❤️

Code climate is point out that this function is a bit complex, and this seems perfect for factoring out to a separate function which returns the next message number (or raises on error).

I'm guessing that if we do throw an exception, we should log that but still try to process the other messages in our queue, so a single bad message doesn't knock out the entire SMS queue. (This isn't a blocker for this PR, but indicates future work that may be required.)

I have no idea what the best way is to log an error from Lambda.

Yep, no issues on separating out into a separate function. I'll do both that and what you're suggesting with error catching as well.

For logging errors from Lambda, I'd use Sentry; it's a really well-thought-out tool with a generous free tier, and the whole thing is open source too! (We run the open-source version on-prem at work.)