Home

Cherish bugs

Note: Meant for programmers. Excuse me for the typos and verbosity. It was a long day and I want to go to bed.

Bugs suck. But they don't have to.

They can be a trigger for self-reflection. If you do a proper post-mortem you learn a lot from them.

For example, I discovered a bug in my code today. I had a section of code meant to fetch the corresponding message (aka. outgoing_message in the code below) to an incoming message from someone. The code in question was:

// BAD
// ...
const [ outgoing_message ] = await strapi.query('outgoing-messages').find({
      created_at_lt: created_at,
      _sort: 'created_at:desc',
      _limit: 1
    })
// ...

It is obvious – taken in isolation – what's wrong with the code.  I forgot to specify who's outgoing message I'm interested in.

// ...
const [ outgoing_message ] = await strapi.query('outgoing-messages').find({
      created_at_lt: created_at,
      to: incoming_message.from_user.id, // <- fix
      _sort: 'created_at:desc',
      _limit: 1
    })
// ...

So how did this happen? I identified at least 5 reasons (apart from probably being asleep when writing this piece of code):

  • Lack of tests
  • Lack of documentation
  • Lack of composition
  • Querying too broadly
  • Bad fixtures on my local machine

Lack of tests

I would have caught this bug if I had taken time to write tests to cover the main contexts in which this piece of code would run.

Lack of documentation

If I would have documented this query I would probably have noticed the gap between the intended behaviour and the implementation.

Lack of composition

If I would have split out this piece of code in a separate method then I would have probably noticed the gap between the intended behaviour and the implementation.

// ...
const get_corresponding_outgoing_message = async () => await strapi.query('outgoing-messages').find({
      created_at_lt: created_at,
      _sort: 'created_at:desc',
      _limit: 1
    })[0]

const outgoing_message = await get_corresponding_outgoing_message()

Having a method called get_corresponding_outgoing_message without an input screams bug.

Querying too broadly

If I followed the principle of querying as precisely as possible (a Yann invention as far as I know; plz don't sue) then I would probably not have made this mistake.

I illustrate this principle with a contrived example: I have a collection Foo and I want to query the latest foo for a given user. In MongoDB this query would look like:

const foo = Foo.findOne(
 { user_id: given_user.id }, 
 { $sort: { created_at: -1 } }
);

Now here I should pause and consider the principle of querying as precisely as possible (please help me come up with a better name):

  • Do I need a specific type of foo of the given user?
  • Does the foo have an organisation and does user have an organisation as well and am I interested in the foo of the given user for a given organisation?

Let's say that the answers are:

  • I need a foo of type "bar"
  • Yes foo has an organisation attribute and user as well and yes I am interested in the foo of the user for a given organisation

Then the query should be:

const foo = Foo.findOne(
 { 
   user_id: given_user.id,
   organisation_id: given_user.organisation_id,
   type: "bar"
 }, 
 { $sort: { created_at: -1 } }
);

This principle – ideally – will trigger you to think through the intended behaviour of your code.

Bad fixtures on my local machine

When I implemented this code I had a database that was pretty much empty. This meant that I was testing under unrealistic conditions. I would have probably caught this bug if I had more data in the database on my local machine.