fix(lock): fix early release of lock

[kishore03109]

Problem

Currently, we are releasing the lock before the promise gets resolved, which then leads to the lock becoming effectively useless as it does not guarantee that each mutation on the repo is isolated from one another. This would prob lead to more users complaining that there are concurrent editors, but there might be further optimisations that could be done to reduce the frequency of this.

Solution

dont release lock early

Breaking Changes

• Yes - this PR contains breaking changes
  • Details ...
• No - this PR is backwards compatible with ALL of the following feature flags in this doc

Before & After Screenshots

BEFORE:

AFTER:

Tests

[ ] Go into any site and perform CRUD operations on a page level for a repo in efs.

[kishore03109]

• fix(lock): fix early release of lock #1256 👈
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @kishore03109 and the rest of your teammates on Graphite

[timotheeg]

LGTM for a very targeted fix 👍

As discussed, a few things are "dirty" and could be refactored.

• We're mixing functional promise chains with .then()/.catch() and async/await style which make things confusing
• The express control flow is not being hijacked correctly: next is supplied to routeHandler(), which could call it, and so we calling next() again later (say, if we cannot release the lock for example) might lead to inconsistent behaviour. As a wrapper to a route handler, the code should take over management of the middleware sequence.

The above issues have potential for some run-time errors, but it looks like in practice, the system has been behaving ok 🤞😰

[timotheeg]

Argh, I just spotted we need to do the same for attachWriteRouteHandlerWrapper(), it has the same early release issue.
I'll open a PR for it now, and we should release urgently too.