Race condition when using relation methods

[aadamcik]

Package version

21.8.0

Describe the bug

If you call any relation method multiple times with slight delay, you will get an error with message: Transaction query already complete.

Example

In our case we were fetching an array of photos from remote server and then saving them to database.

class Property extends BaseModel {
  @hasMany(() => PropertyPhoto)
  declare photos: HasMany<typeof PropertyPhoto>
}

class PropertyPhoto extends BaseModel {
  @column()
  declare propertyId: string

  @column()
  declare file: object
}

const photoUrls = extractPhotos() // returns list of urls of photos

await Promise.all(photoUrls.map(async (url) => {
  const file = await savePhotoToDisk(url) // downloads and saves the photo to disk
  
  return await property.related('photos').create({ file })
}))

As soon as there are several photos to save, there is high chance you will get a race condition, because:

1. Item A before being saved to database will try to create a new transaction.
2. As soon the A acquires transaction it will set it to parent.$trx.
3. An insert query will be sent to DB for item A.
4. Item B will not create a new transaction because parent.$trx is present and use that one.
5. Item A completes the insert and commit transaction.
6. Item B will throw error when trying to send INSERT query, because the transaction was closed.

https://github.com/adonisjs/lucid/blob/21.x/src/orm/relations/has_many/query_client.ts#L103

Reproduction: aadamcik@3c3457b

Reproduction repo

aadamcik@3c3457b

[thetutlage]

I see, this could be problematic and the current workarounds could be:

• Self create a transaction before the loop and use that for all the relationships (even recommended)
• Run a for of loop

Since this is a fundamental change, I will address it in the next major (already in progress)

[aadamcik]

This bug is related to #1095 and just wanted to show that it's not the best solution to rely on $trx in parent model.

1. I don't want to create a transaction before the loop, because that way the download of photo (HTTP request) would be inside the transaction and block it until the photo downloads.
2. This is much slower than downloading and saving it in parallel.

So the only solution right now is to avoid all relation helpers and call PropertyPhoto.create.

These issues are super hard to debug, as it does not happen always and you only get that generic transaction error, which causes a lot of headaches in production.