9

Below is a transcript from work Slack today. Only the names and some code are changed. It ended up causing a bit of drama. DevRanters, what do you take from this?

---

Delivery Lead:
Hey Gang. What's the blocker for FEATURE-123?

Dev1:
FEATURE-122 crashed on iOS app when viewing Feature Introduction page.

Teach Lead:
I've talked about this with Dev1 on a side channel.
And diagnosed the stack trace.
It looks like there is/was some bad handling of a List in the Feature Introduction view logic.
But this is confined to changes that Dev2 is still working on.
(It's not present in master)
Dev2, what's your current position on this?

Dev2:
I have tested at my end with Dev1 but it seems to be working fine

Tech Lead:
There is a race condition related to the use of someList.first()
My guess is that theres a Flow of those lists defined, with an initial value of emptyList
And that on your machine, that Flow is updating with a new value quickly enough that it doesn't matter.
But on Dev1's, for whatever reason, it doesn't get there in time, hits the empty list and falls over.
The logic that's performing the first() needs to gracefully handle empty lists as well.

Dev2:
Where is that logic called?

Tech Lead:
Here's the stack trace Dev1 provided in our conversation earlier:
Caused by: kotlin.NoSuchElementException: List is empty.
...
at 3 iosApp 0x00000000 kfun:kotlin.NoSuchElementException#<init>(kotlin.String?){} + 00
at 4 iosApp 0x0000000 kfun:kotlin.collections#first@kotlin.collections.List<0:0>(){0§<kotlin.Any?>}0:0 + 000
...
at 9 iosApp 0x0000000 kfun:kotlin.coroutines.native.internal.BaseContinuationImpl#resumeWith(kotlin.Result<kotlin.Any?>){} + 0000

This line:
kfun:kotlin.collections#first@kotlin.collections.List<0:0>()
...says that it's first() being called on an empty list.

Dev1:
FYI: Dev3/Dev4/myself are seeing the same issue with the same stack-trace above.

Tech Lead:
So Dev2, have you introduced such a call?
Because I checked master branch and there isn't one, in that version of the file.
Ok, I'll check your working branch Dev2
...
Yes you have here:
var processed1 = someList.first()
var processed2 = someList.first()
...
Lines 123, 124.
Solution looks really straightforward guys.

Dev2:
Okay, I will fix that and push the change

Tech Lead:
Check if someList is empty and allow for generating / handling null processedValues in the view.

Now; I'm going to be straight with you here.
This issue has been discussed over several hours today.
I expect that either one of you could have gone through the process I did in the last 10 minutes above, and resolved it in the same way :point_up:

Dev2:
I went on a break and it's not reproducible on my machine

Tech Lead:
I didn't reproduce it on mine either.

Dev1:
Dev2 and myself are now on sharing screen to sort this issue out. Hope to update back later.

Tech Lead:
<Screen shot of diff with changed code>
:point_up: That change should do it.

Dev2:
Already have pushed the change.

Tech Lead:
...just seen it, is good - same approach :ok_hand:
Dev1 please let us know when tested on your machine.

Dev1:
That does it. It fixes the issues. Thank you, Dev2. I will pick it off from here.

Tech Lead:
Glad to hear it guys.

Dev1:
I have to say this that it is not because we are not working on the issue - Dev2 and myself (together with Dev3/Dev4) have been on this issue all this morning. It just difficult to connect the dot when it wasn't reproducable on Dev2's machine. I brought the issue up because I wanted to switch to working on other tickets while waiting for this to resolve. Still thank you largely for Dev2's work and your keen eyes that spot and resolve the issue quickly.

Tech Lead:
Noted Dev1.
I think the take-away has to be to read the stack-trace carefully... don't worry - we've all been guilty of not reading the error in full, at some point.
The stack trace said that the 'first' element is being referenced from an empty list - that's just logically impossible, right?
Looking for that call to first, we saw it wasn't in the code before, and is after (two of them, in fact).
So then we ask ourselves, how can we deal with an empty list - and then solution almost presents itself.
It didn't really take reproduction of the error to resolve.
Maybe working with a new tech stack creates an anxiety that every issue faced will have a complex solution related to that stack; but I think you'll agree, this particular issue really just required a deep breath and your trusty 'debugging skills 101'... don't lose them! :smiling_face:

Comments
  • 0
    I like the takeaway from that story. Also got the impression that reading isn't the first thing that's done (guess it comes after searching on SO)
  • 0
    Seems fine I guess?

    I don't really like rushing bugs, or judging complexity of bugs without seeing them myself
  • 0
    Thanks for comments.

    Following the above; Dev1 was unhappy that the Tech Lead had said:

    "Now; I'm going to be straight with you here.
    This issue has been discussed over several hours today.
    I expect that either one of you could have gone through the process I did in the last 10 minutes above, and resolved it in the same way"

    Do you think it was fair of the Tech Lead to say this?
  • 0
    Here's what I got from the chat above:
    Dev1 through 4 were already aware of the issue. The only reason tech lead was involved was because they asked about the blocker for 123.

    Had they not commented, the issue would have been solved regardless.

    I understand Dev1's perspective. It doesn't seem like Tech Lead did anything to solve the problem, or at least weren't essential to solving it. Saying "this is how you solve it" is a bit unfair.

    This happens to me all the time, tech lead joins our PS zoom and asks for progress, we give her the latest and tell her we're currently "solving problem X," they proceed to propose a solution for X and assume they just unblocked us when in fact we were already aware of X and working on it, we just had A, B, C ... W to deal with before we could deal with X. We need to remind her that our progress update is not a request for assistance, it's just that, a progress update.
Add Comment