Then There's a Commit Message That Goes Along With It

Steph talks about binging a few Things Worth Learning podcast episodes and particularly enjoyed an episode that featured one of thoughtbot's design directors, Sameera Kapila. Sam shared her expertise about management and inclusion, and Steph shares her favorite parts.

Chris shares the story of a surprising error and the resulting journey through database transactions and Sidekiq that eventually resolved the issue. He also shares some follow up on the broken build and the merging process changes they introduced (spoiler, the process changes have been rolled back).

• Leading Inclusively, with Sameera Kapila - Things Worth Learning Podcast
• How to Skim a Pull Request
• Isolator
• after_commit_everywhere
• time_for_a_boolean

Transcript:

STEPH: Oh man, I'm about to stop eating my pop-tart. I'll put it away. It's within distance. I'm going to eat it.

CHRIS: Your high-fat content unfrosted pop-tart.

STEPH: You know, surprise Sunday twist: it has icing on it.

CHRIS: Steph, who even are you?

STEPH: [laughs]

CHRIS: There are a few canonical anchor facts that one knows about other people, and when one of those...

STEPH: I like to keep everyone, including myself, on their toes.

CHRIS: Or you've just secretly accepted that the icing adds another textural flavor adventure component. It's just better with icing.

STEPH: All right, all right, all right. There's a complicated answer to this. And the complicated [chuckles] answer to this is that the more organic ingredients that I recognize when reading about pop-tarts are by a particular company, and they all have frosting on them. And the more generic pop-tarts that don't have frosting on them, I don't know how to pronounce a lot of those ingredients. So I'm like, no, but okay, I still eat them. But I prefer the ingredients I can pronounce. So I either go with the ingredients I can't pronounce or have a little bit of frosting on my pop-tart. And I'm going with the non-cancer route for today.

CHRIS: For today, in this moment, and accepting the frosting. Okay, all right. Well, that is complicated. [laughs] It's tricky out there.

Hello and welcome to another episode of The Bike Shed, a weekly podcast from your friends at thoughtbot about developing great software. I'm Chris Toomey.

STEPH: And I'm Steph Viccari.

CHRIS: And together we're here to share a bit of what we've learned along the way. So, Steph, what's new in your world?

STEPH: Hey, Chris. So the weather, I'm going to talk about the weather for a little bit. [chuckles] It's been almost non-stop rain for the past several days, which is fine. I'm sure it's great for plant life. But it's really hard on my dog Utah because then we can't go outside for our normal walks and playtime. Although he is my four-legged water baby because he absolutely loves water, and puddles, and playing in the rain. So he's very fine with going outside and playing for a long time. But then I have to essentially give him a full-on bath before I want to bring him back in.

So not wanting to have to give him a bath each time, in the spirit of improvising, we started finding more indoor games to play. And I've started teaching him to play hide and seek. And he's not great at it mainly because he will only stay until I'm out of eyesight, and then he will come and find me. And so I have to be really, really fast at finding a hiding spot to like dash around a corner or hide behind the door. But I think he enjoys it because he will find me and then he seems very excited. And we go back, and we play again. And so I just have to work on teaching him to wait a bit longer so I can find better hiding spots.

CHRIS: When you said that, at first, I was like, how did you teach him to hide? But I realize he's only playing the seek part of the game, and you're only playing the hide part of the game.

STEPH: [laughs]

CHRIS: I'm just so used to you exchange roles back and forth. First, you hide, then you seek, and then you switch it up. That would be a lot to get your dog to be like, now I'm going to secretly hide.

STEPH: [laughs] I'd be very impressed. Yes, we have very distinct roles in this game. I am the one that always counts and hides. But he's a very good seeker. So that's been fun. We just got to work on getting a little better at it.

But on a more tech-related note, one of the design directors at thoughtbot, Sameera Kapila, who also goes by Sam, was a guest on the podcast Things Worth Learning, which is hosted by Matt Stauffer. And Matt is also the host of The Five-Minute Geek Show and The Laravel Podcast. And in the show Things Worth Learning, Matt meets with individuals that are excited to share something that they're deeply passionate about; maybe it's tech, maybe it's not. And I've binged a couple of those episodes.

And I really like how you can choose between the podcast format or the YouTube format. So then you can really watch the conversation unfold, which I know you and I a couple of times have thought it would be fun if people could see us because there are so many facial emotions and gestures that go along with conversations. So it was really delightful.

And speaking of delightful, Sam shared her expertise about management and inclusion. And I definitely recommend listening to the episode because I can't share everything that Sam shared. But a couple of the topics that Sam mentioned that I really enjoyed and would love to chat about, so the first one is about helping someone, in this case, someone that you manage that comes to you with a concern.

So there's often a presumption that just because someone comes to you with a concern or an issue that they've experienced at work, that they're the ones that will also want to work to address that concern, and that's often not true. It can be true; maybe that person wants to be involved. But they're often coming to you in the leadership or management role to say, "Hey, I've had this issue," and they really want help with that instead of walking away with homework for it. Because then that trains people to essentially be in this mindset of well, if I bring up this concern, then I'm going to be the one that has to address it, even if I'm the one that's most negatively impacted by this. And addressing this concern could be actively harmful to me.

And she shared a really great real-world example from her own experience where her and another co-worker had noticed a concern about the hiring process. And her and that co-worker got together, and they talked about the concerns. They even rehearsed for the meeting because they were trained by the tech industry to say, "Hey, if you bring up a concern, you're going to be responsible for addressing and then resolving that concern."

And so they had that meeting with the person in leadership. And they were pretty nervous about how it was going to go. And that person in leadership said to them, "Thank you both so much for sharing that. That must have been such a burden. And this is my responsibility to fix. And here are what my next steps are." And that was amazing because it allowed Sam and the other person to go back to client work. And they also received follow-up conversations about how that issue was being addressed. So there was even that feedback loop as to how things were going to change.

And I have a personal example that...I really resonated with the example that Sam provided because I remember there are different teams that I've been a part of, where often I was one of the few women engineers on the team. And so we often have conversations about how do we get more women engineers into the company? And they're wonderful conversations.

But there's a part of me that always felt resentful about, like, why am I here? Why am I the one fixing this? I understand I have some more insight and expertise, and experience in this area. But I was also frustrated by the fact that I was the one that was in that meeting often with other women, and it felt like our responsibility to fix this. And I used to feel bad about feeling resentful towards that. Because I was like, shouldn't I want to help other people? And I do. But Sam's example really helped remind me and clarify that yes, just because there's a concern doesn't necessarily mean you should be the one to address it. And it really takes everybody involved, or it takes leadership to step up and address that concern.

CHRIS: Oh, that's really interesting the way Sam is framing that and describing the situation of not having any problem that you bring in be now your work to solve. Like, oh, I found the issue, and now we've got to go do this. But the idea that you can bring something to light and then be able to walk away from it.

And the particular thing that you were saying that if your interaction is always that when you reference something when you bring in a concern that then your manager works with you to figure out how you can solve it, then you get this mental block of like, well, do I even want to say anything? Because I don't want to try and deal with big, amorphous unclear issues. So maybe I just won't even say anything.

And so this as a way to make sure that there's room for all of the conversation is a really interesting framing that I hadn't really thought about, frankly, but it's very interesting. I haven't seen this interview either. So I'm definitely excited to give this a look because Sam is wonderful. And the topic that you're describing here sounds fantastic as well.

STEPH: Yeah. There was an important moment for me where...one of my managers is Matt Sumner, who's been on the show. And when Matt was my manager, at one point, we were having a one on one, and we would often go for walks for our one on one. And I mentioned something about "I have this concern, or I have this problem, but I don't really know how to fix it. So I'm not sure I'm ready to talk about it." And Matt, in his delightful way, was like, "We can still talk about it. You don't have to have an answer or a solution." I'm like, "Yeah, but I feel like I should be able to fix it. Like, if you have a concern, or if you have something that you want to gripe about, then you should come to the table with solutions for it." And Matt was like, "No, you don't need to do that at all. We can totally gripe about stuff or talk about concerns and then either figure out the solutions together or go to other people for ideas."

And that was really important to me because, like you'd mentioned, otherwise, it felt like this mental block where then it feels like you can't air out some of the things that you're worried about or have concerns about because then you think you're the only one responsible. And you may not be able to come up with the best solution. You may need other people to then help you strategize and come up with ideas. And I just love, love, love that part of Sam's discussion.

And oh, there was one other part about the conversation. Well, there are lots of parts that were amazing. But another one in particular that blew my mind is about Comic Sans, the font, the font that everyone loves to hate. [chuckles] And I learned that it's one of the most legible fonts for kids. And it's one of the more accessible fonts for people with dyslexia. And it's actually recommended...I think there are still more academic studies that need to be done to really classify fonts that are best for people that have dyslexia.

But Comic Sans is recommended by The British Dyslexia Association and the Dyslexia Association of Ireland. And there are some other really great posts that talk about the benefits of using a font like Comic Sans because the typeface has long ascenders and descenders and generous letter spacing and asymmetrical lowercase b and d to then help distinguish those letters. And I just thought that was so cool. This font that everybody wants to rip apart because it seems whimsical, unprofessional gets overused. There are lots of reasons, I suppose. [laughs] But there's a really big benefit to it, and it can help others. And I just found that very whimsical in itself.

CHRIS: I love the idea that there are multiple levels of knowing about Comic Sans. First, you're just like, I don't even know the name, but it's that comic book-looking font. And then obviously, the next step is to be like Comic Sans? How could you ever use that? It's an atrocity. And then it's like, but actually, Comic Sans has some things going for it. And it is a really interesting consideration and something that you wouldn't necessarily think of. But then once you learn it, you're like, okay. Man, I wonder how many other things in the world have this interesting shape to them? Hmm.

STEPH: Do you know the history behind Comic Sans?

CHRIS: I do not.

STEPH: I read about it fairly recently, but I'm probably going to botch some of the details. But I believe it was designed or created by Vincent Connare. And it was created for Microsoft. And Vincent was working on a project where I think there was a dog that was essentially going to have these bubbles that would then show you different parts of the application and walk you through the different features. And the dog had a very comic book feel to the character.

And so then Vincent designed a font to go along with that comic book character, this dog and came up with Comic Sans. I don't think the dog actually launched with that particular font. But since the font was still developed, it was released as part of the available fonts. And there we go, there is the birth of Comic Sans. And then it just received so much love and ire all throughout history. [chuckles]

CHRIS: There's something that you said there that I want to loop back on when you were talking about chatting with Matt Sumner and saying, "Here's this thing, but I don't know how to solve it. So I don't even want to bring it up." I really liked the framing that you gave and the fact that Matt was like, "No, no, we can still talk about it. We can at least explore this thing, have a conversation." I think that's really wonderful.

There's a very similar thing that I experience a lot when doing code review, particularly when I'm in more of a leadership role within a team, which is I often want to highlight something that feels a little bit off to me in the code, but I may not have a specific solution. Like, I may see a variable name, or I may see a controller action that feels like it's the wrong shape or something. And I'll often name it but explicitly say, "I actually don't have a better idea here. So feel free to continue on with this, but I want to name it. So in case that sparks something in you, if you were also feeling some incongruousness, maybe it's worth you spending another minute to think about it, but I want to make sure my comment isn't blocking or otherwise making you feel uncomfortable."

If I just come to you and I'm like, "This feels wrong," and that's all I say, that to me is unacceptable code review. Because now I want all of my code review feedback to be very actionable, it’s either here's the thing that I feel strongly I think we should definitely change this. If you disagree, let's have a conversation. But yeah, this one definitely needs to change. Here's the thing that, like, I don't know, maybe we could break this into two lines and split it up. But if you don't like that, that's fine. Do whatever. And so then it's I've given the person my thoughts but given them clarity and a free rein to do whatever they want with that information.

And then there are ones where I'm like, I don't even know what I think we should do here, but I think something. But if you don't have any ideas...like, I don't have any ideas specifically. If you don't have any ideas, it's fine. We'll continue on with this and maybe revisit it down the road. But I want to make sure each of those different tiers is actionable for the other person, and I'm not just giving them homework or something to be sad about because that would be bad code review.

STEPH: I'm just imagining a PR comment that says, "I don't know what we should do here. But I don't think this is it," [laughs] and that just creating sadness. That's so interesting to me because I have flip-flopped with that opinion in regards to there are times that I very much resonate and do what you just said where I will point out to someone where I'm like, "I'm not sure why, but I just have concerns about this. And I don't know if you also ran into anything that was weird about this and would like to talk about it. I don't have any really great ideas, so I think this is good for now. And we should keep moving forward, so we're not blocked on it," but just wanted to, as you mentioned, highlight it in case it sparks something for the other person or for someone else that's reviewing the code.

And then there are other times where I'll look at something, and I'm like, "Yeah, it's not great. There's something that feels brittle or potentially maybe hard to maintain or things like that. But I don't have a better idea." And I don't comment on it because I'm like, I don't want to distract that person or block them. And I do think it's good enough, and I don't have anything to add to the conversation, so I just leave it out. So it's interesting to me where is that line of when I feel like it's important enough to comment to then potentially spark some conversation versus just letting it go so then I don't add any distraction to their work?

CHRIS: I think it's when the spidey-sense gets past 47%. It's a very specific number. I do the same thing where there's something, and I'm like, you know what? I can't even clearly express what about this makes me feel something off, and so I won't even comment on it, and I agree. And then there are things that trip past some magical line in the sand. And I'm like, you know what? I think I'm going to say something here, but I don't even have a recommendation. And then there's a whole spectrum of the nature of code review and, again, 47% being the specific number.

STEPH: There's actually a thoughtbot blog post that correlates nicely to that concept of spidey sense. It's written by Mike Burns, and it's titled How to Skim a Pull Request. But essentially, grabbing from one of the lines here is where Mike presents an unexplained, incomplete, and arbitrarily grouped list of keywords that will cause us thoughtboters to read your code with more care and suspicion. [laughs] That feels perfectly aligned with that idea of spidey sense, spidey-sense 101. I'll be sure to include a link in the show notes. Or, you know, 40%.

CHRIS: I think it was 47%. It's a very precise number. [chuckles]

STEPH: Very precise nonsensical number. Got it. [laughs]

CHRIS: If I'm making up fake statistics, I'm not going to have them round to an even 10. [laughter]

STEPH: Makes it seem more legit somehow.

CHRIS: Exactly.

STEPH: But that's really the novelties that I wanted to chat about.

Mid-roll Ad

And now a quick break to hear from today's sponsor, Scout APM.

Scout APM is leading-edge application performance monitoring that's designed to help Rails developers quickly find and fix performance issues without having to deal with the headache or overhead of enterprise platform feature bloat. With a developer-centric UI and tracing logic that ties bottlenecks to source code, you can quickly pinpoint and resolve those performance abnormalities like N+1 queries, slow database queries, memory bloat, and much more.

Scout's real-time alerting and weekly digest emails let you rest easy knowing Scout's on watch and resolving performance issues before your customers ever see them. Scout has also launched its new error monitoring feature add-on for Python applications. Now you can connect your error reporting and application monitoring data on one platform.

See for yourself why developers call Scout their best friend and try our error monitoring and APM free for 14 days; no credit card needed. And as an added-on bonus for Bike Shed listeners, Scout will donate $5 to the open-source project of your choice when you deploy. Learn more at scoutapm.com/bikeshed. That's scoutapm.com/bikeshed.

STEPH: What's new in your world?

CHRIS: I have some follow up on a recent topic that we talked about. So we had a kerfuffle which I described where we had a branch that got merged and the rebase some stuff got out of hand. And so we introduced some process, the protected branch configuration within GitHub that required the branches to be up-to-date before they can be merged and CI to be passing. And everybody was happy. It was like, this is great.

Turns out it was never turned on. That's actually the day I was like, man; this is really straightforward. There's been no annoyance here. And then I got to the point where it was like; this seems weird because we just merged a lot of things in rapid succession.

I went and checked, and it turns out what I thought was the name of the branch protection rule in GitHub's UI is, in fact, a regular expression pattern. It might not be a full regular expression but like a wildcard pattern for the branch name to match to, and so it's specific.

I created this rule, and in small, gray text underneath, it said, "This applies to zero branches." I missed that the first time but then the second time going back, I was like, oh, I actually wanted it to apply to more than zero branches. So I went back in and changed that. It's a great example of very subtle UI that just slipped past me.

STEPH: I was going to say in your defense, the very subtle gray font to say, "This applies to zero," feels tricky.

CHRIS: That...also, going through the work of creating this thing and if that results in zero branches that would match, maybe that's the thing to emphasize on creation. I would love that. Because in my case, I was trying very specifically to target an existing branch. There is the ability to say, "Oh, any bugfix-* named branch," if you're using branch naming strategies like that, you can use this for that sort of thing. So it may be that currently, there are no branches with that name. But in my case, I was just like, please, main, anytime anything is happening on main, that is what we want to do. I just needed to put the word main there. But anyway, once I actually turned it on, insufferable, absolutely not, cannot survive in this world.

We have a relatively small team. There are three of us, and not everyone is even full-time, and my time is pulled in a lot of different directions. So I'm actually not pushing as much code as I might otherwise. Even with that, nope, absolutely not. Our CI is like; I don't know, five-ish minutes per run. Turns out, especially Monday mornings, we have a volley of things that will have been reviewed and trickled in through Friday afternoon. And then there's a bunch of work we want to land Monday morning. And then, just at any point, it turns out, yes, this was untenable. So we have turned it off.

I would like to revisit this down the road and introduce the MergeQueue functionality, so the idea of being able to say, "Yeah, you just name when you want something to go in, and then the system will manage the annoying finicky work there." But for now, I had to give up on my dream of everything running on CI, on a feature branch, before it gets merged.

STEPH: Ooph, that phrase, "I had to give up on my dream," that breaks my heart for you. [laughs]

CHRIS: I may be going a little bit fanciful with my language but, like, a little.

STEPH: [laughs]

CHRIS: I liked this thing. I want to exist in that world. But it is not feasible given the current state of the world. And that will only get worse over time, is my expectation. So I get to revisit this when I have the time to more thoroughly figure a thing out. But for now, I don't know, merge whatever; it will be fun.

STEPH: There's a small part of me that feels a little reassured that it was a terrible time, although I hate that it was a terrible time. But I have felt that pain on so many other projects where I am constantly waiting, and I'm constantly checking to be like, can I merge? Can I merge? Can I merge? And then I can merge, but then someone beats me to it. And I'm like, oh, then I got to restart. And I got to wait, and I'm constantly checking. So that feels like it helps validate my experience. [chuckles]

I am excited for that MergeQueue. I would be super excited to try that out and hear about how it goes just because that seems more like the dream where you can just say, hey, I want this PR to go whenever it can go. Just take care of it. I want it to be rebased, whatever the flow is, and have it be merged, so I don't ever have to check on it again.

CHRIS: But once we configured this, there was a new thing that appeared in the GitHub UI, which was auto-merge. And so that was a button where I could say like, "Hey, merge this whenever CI passes," which was a nice upgrade, but it didn't have the additional logic of and rebase as necessary. Or the more subtle logic of like, you don't actually want to rebase where you have five different branches that are all trying to merge, and they keep rebasing. You want to have the idea of a queue, and so you get in line. And you rebase when it's your turn, and then you run the CI. And you try and be as smart as possible about that.

If anyone at GitHub is listening, I would love if you all threw this into your platform, and then you could ping Slack if anything went wrong. But otherwise, there are, like I said, existing tools. At some point, I will probably, I don't know, over a long weekend or something like that, sit down with a large cup of coffee and explore these. But today is not that day.

STEPH: I'm excited to hear about that day.

CHRIS: So that is a tale of woe and sadness. But luckily, I get to balance it out with a tale of happiness and good outcomes. So that's good. The happiness and good outcome story does start with trouble, as they always do. So we had a bug that occurred in the application where something was supposed to have happened. And then there was an email that needed to go out to tell the user that this thing had happened. And the bug popped up within AppSignal and said something was nil that shouldn't have been nil.

Particularly, we're using a gem called Time For a Boolean, which is by Caleb Hearth. And he's a former thoughtboter and maintains this wonderful gem that instead of having a Boolean for like, is this thing approved, or is it paid? Or is it processed? You use a timestamp. And then this gem gives you nice Boolean-like methods on top of that timestamp. Because it turns out, very often just having the Boolean of like, this was paid, it turns out you really want to know when it was paid. That would be a really useful piece of information. And so, while you're still in Postgres land, it's nice to be able to reach for this and have the affordances of the Boolean-like interface but also have the timestamp where available.

So anyway, the email was trying to process but that timestamp...let's pretend that it was paid as the one that matters here so paid at was nil, which was very concerning. Because this was the email that's like, hey, that thing was processed. Or let's say it was processed, actually, because that's closer to what it was. Hey, this thing was processed, and here's an email notification to tell you that. But the process timestamp was nil. I was like, oh no. Oh no. And so when I saw this pop up, I was like, this is very bad. Everything is very bad. Oh goodness.

Turns out what had happened was...because I very quickly chased after this, looked in the background job queue, looked in Sidekiq's UI, and the job was gone. So it had been processed. I was like, wait a minute, how? How did this fix itself? Like, that's not the kind of bug that resolves itself, except, in this case, it was. This was an interaction that I'd run into many times before. Sidekiq was immediately processing the job. But the job was being enqueued from within the context of a database transaction. And the database transaction had not been committed yet. But Sidekiq was already off to the races trying to process.

So the record that was being worked on, the database record, had local changes within the context of that transaction, but that hadn't been committed. Sidekiq then reads that record from the database, but it's now out of sync because that tiny bit of Sidekiq is apparently very fast off to the races immediately. And so there's just this tiny little bit of time that can occur. And this is also a fun one where this isn't going to happen every time. It's only going to happen sometimes. Like, if the queue had a couple of other things in it, Sidekiq probably would have not gotten to this until the database transaction had fully closed.

So the failure mode here is super annoying. But the solution is pretty easy. You just have to make sure that you enqueue outside of the database transaction. But I'm going to be honest, that's difficult to always do right.

STEPH: That's a gnarly bug or something to investigate that I don't think I have run into before. Could you talk a little bit more about enqueueing the job outside the database transaction?

CHRIS: Sure. And I think I've talked about this on a previous episode a while back because I have run into this one a few times. But I think it is sufficiently rare; like, you need almost a perfect storm because the database transaction is going to close very quickly. Sidekiq needs to be all that much more speedy in picking up the job in order for this to happen.

But basically, the idea is within some processing logic that we have in our system; we find a record, we do some work. And then we need to update that record to assign this timestamp or whatever it is. And then we also want to inform the user, so we're going to enqueue a job to send the email notification. But for all of the database work, we are wrapping it in a transaction because we want it to either succeed or fail atomically. So there are three different records that we need to update. We want all of them to be updated or none of them to be updated. So, therefore, we wrap it in a transaction.

And the way we had written, this was to also enqueue the job from within the transaction. That wasn't something we were actively intentionally doing because those are different systems. It doesn't really mean anything. But we were still within the block of ApplicationRecord.transaction do. We're now inside of that block. We're doing all of the record updates. And then the last piece of work that we want to think about is enqueueing the job to send the email.

The problem is if we're still within that database transaction if it's yet to be committed, then when Sidekiq picks up that job to run it, it will see the prior state of the world. And it's only if the Sidekiq job waits a little bit that then the database transaction will have been committed. The record is now updated and available to be read by Sidekiq in the correct updated state.

And so there's this tiny little bit of inconsistency that can happen. It's basically because Sidekiq is going out to Redis, which is a distinct system. It doesn't have any knowledge of the database transaction at play. That's why I sometimes consider using a Postgres-backed background job system because then actually the job can be as part of the database transaction.

STEPH: Cool. That's helpful. That makes a lot of sense the way you explained the whole you're actually enqueueing the job from inside that transaction. I'm curious, that prompts another question. In the case where you mentioned you're using a transaction because you want to make sure that if something fails to update so, everything gets updated together, in the event that something does fail to update because you were previously enqueueing that job from the transaction, does that mean that the update could have failed but that email would still have gone out?

CHRIS: That does not. And the reason for that is because we're within dry-monad world. And so dry-monad will implicitly capture the ActiveRecord rollback, which I think is an exception that gets raised or somehow...But basically, if that database transaction fails for any reason and ends up getting rolled back, then dry-monads will not continue processing through the rest of the sequential operation. And so, therefore, even if we move the enqueuing of the email outside of the database transaction, the sequential nature of that processing and the dry-monad stuff that we have in play will handle that. And I think that would more generally be true because I think Rails raises an exception on rollback. Not certain there. But I know in our case, we're fine on that. And we have actually explicitly checked7 for that sort of thing.

STEPH: So I meant a slightly different question because that makes sense to me everything that you just said where if it's outside of the transaction, then that sequential order won't fire because of that ActiveRecord migration error. But when you have the enqueuing inside of the transaction because then that's going to be inside of the sequential order, maybe before the rollback error gets raised. Does that make sense?

CHRIS: Yes. I think what you're asking is basically like, do we make sure to not send the job if the rest of the stuff didn't succeed?

STEPH: I'm just wondering from a transaction perspective, actually. If you have a transaction wrapped block and then you have in there, like, update this record, send email, end block, let's say update...well, I guess it's going raise because you've got probably like an update bank. Okay, so then yeah, you won't get to the next line. Got it. Got it. Got it. I just had to walk myself through that because I forgot that you probably...I have to visualize [laughs] as to what that code probably looks like. All right, that answered my question.

CHRIS: Okay. So back up to the top level then, this is the problem that we have. And looking through the codebase, we actually have it in a bunch of different places. So the solution in any one of those cases is to just take the line of code where we're saying enqueue UserMailer.deliver_later take that line of code, move it outside of the database transaction, and make sure it only happens if the database transaction succeeds. That's very easy to do in one case.

But my concern was this is a very easy failure mode to end up in. And this is a very easy incorrect version of the code to write. As far as I can tell, we never want to write the code where this is happening inside of the transaction because it has this failure mode. But how do we enforce that? That was the thing that came to mind. So I immediately did a quick look of like, is there a RuboCop thing I can do here or something?

And I actually found something even more specific, which was so exciting to find. It's a gem called Isolator. And its job is to detect non-atomic interactions within database transactions. And so it's fantastic. I was like, wait, really? Is this going to do the thing? And so I just installed the gem, configured it where I wanted, and then ran the test suite. And it showed me every place throughout the app right now where we were doing this pattern of behavior like enqueueing work from within a database transaction, which was great.

STEPH: Ooh, that's really nifty. I kind of want to install that and just run it on my current client's codebase and see what I find.

CHRIS: This feels like something like strong migrations where it's like, yeah, this is great. I kind of want to have this as part of my core toolset now. This one feels even perhaps slightly more so because sometimes I look at strong migrations, and I'm like, no, no, no, strong migrations, I get why you would say that, but for reasons, this is actually fine. And they have configurations within it to say, like, no, this is okay. Isolator feels like it's always telling me something I want to know. So this, very quickly, I'm like, I think this might be part of my toolset moving forward on every single app forever.

And actually, there's another gem that I used. It's made by the same team. So this is from the folks over at Evil Martians, which is another Rails consultancy out there in the world. And the Isolator gem is one thing that they've produced. And then I think the same author of it who is an Evil Martian's employee created the after_commit_everywhere gem.

So after_commit is one of Rails' ActiveRecord callbacks. But in this case, it allows you to use it everywhere, as the name implies. And so rather than actually having to take that line of code out of the database transaction block, which is naturally where we would write it because that's how we think about the code and how we want to express it, you can just use this after_commit method, wrap the call in that, so it's after_commit, and then a block. So either braces or do..end. That enqueueing of the email now just gets wrapped in that. And so what that does is it says, "Defer this until after the transaction commits. If the transaction does not commit, if we roll it back, then don't run it."

And what was nice is the actual code change when I finally submitted all of this was add the gem to the gem file. And then everywhere that we're doing the wrong thing, which running the test suite told me, I just went in, and I wrapped that line in after_commit and a block. And it was such a nice, clean...like, I didn't have to move the code around or actually shift the lines, which was my first attempt at this. I was able to just annotate each of those lines and say, "You're special, you're special, you're special," And then I'm done. And again, the first gem told me every case where I needed to do that. It's like, well, this is a wonderful little outcome here.

STEPH: That's really nice, yeah, how you can make the changes and then, like you said, re-run the test or re-run that gem, and it lets you know what else still needs to be updated. I'm intrigued where you mentioned you didn't have to move any lines, though. Maybe I just need to look at the gem and see it, but I'm still envisioning that you have your transaction do block. And then you're doing some things; you're updating records, and then you have your end. And then after that, it's when you want to enqueue the email. And with this after_commit, you actually added that method call inside of the transaction but then wrapped the call to Sidekiq to send the email inside of that block.

CHRIS: Correct. Yeah. So it's basically like saying, "Here's almost an anonymous function." If you think about a Ruby block in that nomenclature, you're saying, like, here's some work to do when and if the transaction succeeds. And so it meant that I was able to keep the code in the way that we as humans would talk about it but deal with the murky details, and edge cases of database transactions, and Sidekiq, and whatnot. Sort of just handle it by saying like...it almost feels like an annotation or a decoration or something like that. But it was this, in my mind, almost like a perfect melding of I don't want to think about this. Oh, cool. Okay, here's a quick, easy way to deal with it but to not have to fundamentally change how I write the code.

STEPH: Interesting. So I like all the things you're saying. I'll be honest, I'm not totally sold, and I'm trying to think of why. I think the benefits...one, as you mentioned, it's something you don't have to think about or at least signals to others that hey, maybe you should think about this to the extent that you use after_commit. And so that way, you don't have these asynchronous events taking place inside the transaction. So I like that visibility and communication to the rest of the team. Putting it inside of the transaction feels interesting. I don't know why; I feel a little weird about this. [laughs] I'm bringing my true self.

CHRIS: That's fair. So if we're being honest, I solved this first by finding the Isolator gem. Well, I solved it first by just doing it manually. I went through the app, and I found all the places. And I was like, you know what? I'm worried that the next person authoring code like this, it's so easy to fall into this trap. Like, this is such a subtle little thing that our brains are not thinking about. And so I had first fixed it, and so I had a diff that involved moving lots of lines of code, every instance of this moved from being in the database transaction out of it. And that was fine. I was fine with that as a solution. But it was a little bit noisy because I was moving a bunch of lines.

So then I brought in the Isolator gem. I actually reset that, and I went back to before I had made the fix, ran the test just to make sure Isolator was actually finding every instance. They did; that was great. So I was like, all right, cool. This is better because now I have this thing that will tell anyone when this happens. So I'm very happy about that. Because frankly, this is some hard-earned knowledge that I had to read Sidekiq and remember how database transactions work and convince myself of what was going on here and finally come to what I believe the solution is.

And now Isolator is just like, cool, that's encapsulated. And it gives a very nice failure message in the test suite. So it's like, excellent. I really like this. But still looking at it, the diff, the amount of code that I had to change, it's like, well, naturally, this is how we want to write this code, but for reasons, we can't. And it's appeasing the computer more than it's appeasing the reader or the author of the code.

And so then I happen to be reading through the Isolator gem's README, and they mention the after_commit_everywhere gem. And I was like, oh, that's interesting. So one more time, I reset. And then I really tried fixing it with after_commit. And the look of the diff there felt nice to me because the lines got a little more on them, but they didn't move. And so it's like, this is how we naturally would have authored it, and now it works correctly. And I liked that.

But I understand your hesitation because you're like, but the thing is, it's wrong. And so you've made the wrong not wrong anymore, but you didn't...and so I get your hesitation. I still like the fancy version.

STEPH: Yeah, I think you just helped me figure out my grumpiness with it or why I'm not totally sold on it. And it was in regards to adding a dependency to avoid a noisy diff is the oversimplified version that I was processing or the reason that I was a bit grumpy about adding this other gem for that. But then you also just brought a lot of other really good reasons.

One thing that you said that I do really like is adding tools that help us author code in a more natural style, the way that we want to highlight this process, and how this application does work, and how this business logic flows. So given in that light, that makes me feel better about it. But yeah, I think that was my initial grumpiness. I was like, it’ll be a noisy diff. It's okay.

CHRIS: I think I definitely share your hesitation, or you're like, hmm, that's an interesting reason to bring more code into the application. But at the same time, I think the counterpoint that comes to mind for me is we're using Ruby because of its expressiveness; at least, that's why I'm using Ruby. I really want the code that I write to be as close as possible to the thing that I would say to another human about like, oh okay, when a user signs up for the application, we need to create a record in our system, and then we need to send them an email. And then we need to do this other thing. And so, the closer that our code is to those words that I would use to describe to another human, the happier I am.

And I will put in some pretty significant effort to hold that line as long as the code can also be correct. And so, the Isolator gem here does a great job of enforcing that correctness. And then after_commit allows me to still maintain that expressiveness and not have to think about the murky details as much or not have to reshape my code to match the murky realities of different persistence engines.

But I do agree. I think it's a good thing to look at and ask, like, is it worth it? Are you sure? And in this case, I will say, "Yeah, I think so," but with that amount of certainty in my voice, [chuckles] which is not a ton.

STEPH: I think this is going back to my days of working with dependency bot PRs where every time there was an upgrade for a gem, I always ask, what do you do here? [chuckles] Do we need to upgrade you? Can we just remove you from the codebase? So I'm fairly...I don't know, resistant is a strong word. I'm skeptical of when we're adding stuff in, and I just want to question the value that it's adding.

But I want to circle back to something that you said, and that is hard-earned knowledge. And that part I understand so much where when you have gone through a fair amount of work to uncover an issue, and then you want to make sure that others don't have to go through that. This is a really nice way to highlight; hey, there's something that's tricky about computers and software here, and we need to watch out for that. And I want to help you lookout for that. Versus this is just inherit information where this needs to happen outside or after that transaction. And so that makes a really nice entry point where someone can look to say, "Why did we add this gem?" And then there's a commit message that goes with it that explains this is why we use this after_commit gem because we're specifically looking to avoid this type of bug. And I love that.

CHRIS: Yeah, I think more lines of git commit message than diff on this one. So yeah, I wrote a short novel describing all of the features, describing the different pieces that are coming together. And then it's actually a +28 -6 diff. So it's a very small code change. But yeah, lots of story captured there.

STEPH: And if you had just moved the lines, you could still have that commit message. But it's not likely that someone's going to look up that git commit change or that message that went along with it because they're not going to know to blame that one. But if they look at that particular edition of after_commit, they're more likely to find that historical context. So long story short, I think you have walked me through my initial grumpiness and provided some really good ways to avoid that really tricky failure mode for other developers.

CHRIS: Well, thank you. I'm getting Steph's seal of approval starting from grumpy places. [laughs] I feel good. All right.

STEPH: I'll have some special Stephanie's approval stickers designed and printed for you.

CHRIS: I hope you're not joking because I very much want a yellow heart that says, "Steph-approved."

STEPH: [laughs]

CHRIS: And I can put it on PRs, and I can put it on the wall. [laughs]

STEPH: Well, now I have to find a sticker designer and make a...well, it's just a yellow heart. I can probably handle this. I'm going to use Comic Sans. That will be the approved part. [laughs] Yellow hearts and Comic Sans for everybody.

CHRIS: Well, with that absolutely fantastic call back to earlier parts of the episode, shall we wrap up?

STEPH: Let's wrap up.

CHRIS: The show notes for this episode can be found at bikeshed.fm.

STEPH: This show is produced and edited by Mandy Moore.

CHRIS: If you enjoyed listening, one really easy way to support the show is to leave us a quick rating or even a review in iTunes, as it really helps other folks find the show.

STEPH: If you have any feedback for this or any of our other episodes, you can reach us at @_bikeshed or reach me on Twitter @SViccari.

CHRIS: And I'm @christoomey

STEPH: Or you can reach us at hosts@bikeshed.fm via email.

CHRIS: Thanks so much for listening to The Bike Shed, and we'll see you next week.

All: Byeeeeeeee!

Announcer: This podcast was brought to you by thoughtbot. thoughtbot is your expert design and development partner. Let's make your product and team a success.

Sponsored By:

• Scout: Scout APM is leading-edge application performance monitoring designed to help Rails developers quickly find and fix performance issues without having to deal with the headache or overhead of enterprise-platform feature bloat. With a developer-centric UI and tracing logic that ties bottlenecks to source code, you can quickly pinpoint and resolve performance abnormalities -- like N+1 queries, slow database queries, memory bloat, and more. Scout's real-time alerting and weekly digest emails let you rest easy knowing Scout's on watch and resolving performance issues before your customers ever see them. Scout has also launched its new error monitoring feature add-on for Python applications. Now you can connect your error reporting and application monitoring data on one platform. See for yourself why developers worldwide call Scout their best friend and try our error monitoring and APM free for 14-days, no credit card needed! And as an added bonus for Bikeshed listeners: Scout will donate $5 to the open-source project of your choice when you deploy. Learn more at scoutapm.com/bikeshed.

Support The Bike Shed