This content originally appeared on Jonnie Hallman (@destroytoday) and was authored by Jonnie Hallman (@destroytoday)
While clicking through my Cushion account yesterday, I noticed that my account’s preference for the weekly summary email was enabled, but I didn’t recall receiving one recently. At first, I wondered if I accidentally stopped the weekly scheduled job that sends all the emails, but no—it’s still there. Then, I double-checked the code to make sure that the query for the weekly summary users was correct—it was. Finally, I looked at Cushion’s logs for when the job runs. I saw where the job started, then I saw a stacktrace. This is where I slowly looked up from my computer like that Bert & Ernie gif.
The error indicated that Time.zone, which I reference to get the relevant time to send the emails for each user, is null. There’s two strange parts to this. First, why is it null? And second, why didn’t I catch it?—either in the tests (that all pass) or from Sentry, Cushion’s exception tracker. I couldn’t recall making any recent changes to this code, but nevertheless, I checked its git history.
Apparently, in August of 2019, I updated weekly summaries to no longer be sent to “expired” users, which included folks whose trial expired or subscription ended. I did this because I’d occasionally get emails from people rightfully complaining, but also, these emails were taking huge chunks of email credits out of my Postmark account. At the time, I was looking to cut back on expenses, so this seemed like an opportunity to feed two birds with one scone. I updated the condition to check for the user.expired? method, which relies on Time.zone, and wrote tests to validate that it all works. I deployed the update and watched as my Postmark credit no longer dwindled—success! What I didn’t do was verify whether Cushion still actually sent the weekly summaries in production.
Why doesn’t this work? Well, it’s easy once you know. Time.zone is set whenever Cushion authenticates a user, using their timezone. If I’m querying an array of users in a background job, where I don’t need to authenticate a specific user, I don’t ever set Time.zone. Therefore, it’s null. The tests pass, however, because I do set Time.zone to UTC in the test environment setup.
I attribute the brunt of this misstep to 2019 me being a weak version of myself with rusty Ruby skills. If I were to code this today, with a fresh and healthy mind, I would’ve simply used the user’s timezone property that comes with the query. In any case, here’s the embarrassing part—not only has this bug prevented weekly summaries from going out for exactly one year, but on top of that, no one mentioned that they no longer received their weekly summary.
Glass-half-empty me would let this tear me apart, but optimistic me sees this as a positive signal—if you take something away and nobody notices, did they even need it in the first place? Sure, it’s a nice-to-have feature and receiving one less email each week could go unnoticed, but when I’m already looking to cut the fat in Cushion, this is good news. I still believe some sort of weekly summary is useful, but I want it to be a crucial feature that users rely on versus one that nobody misses when it’s gone.
This content originally appeared on Jonnie Hallman (@destroytoday) and was authored by Jonnie Hallman (@destroytoday)
Jonnie Hallman (@destroytoday) | Sciencx (2020-08-05T08:00:00+00:00) Well, this is embarrassing. Retrieved from https://www.scien.cx/2020/08/05/well-this-is-embarrassing-2/
Please log in to upload a file.
There are no updates yet.
Click the Upload button above to add an update.