Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage as middleware in an express app with lots of async stuff #89

Open
NavaceSystem opened this issue Aug 14, 2024 · 0 comments
Open

Comments

@NavaceSystem
Copy link

Hi, I'm seeing some weird "context leaking" in my express app where occasionally, 1 user seems to get the data of another user and I just want to confirm if my usage of cls-hooked below is correct. Some bits are omitted/shortened for simplicity.

  • The /some-mutation endpoint, starts a database transaction that spans the request, calls a bunch of domain functions that update the database and then calls commit/rollback on the transaction.
  • getDBConn returns a db connection from request context if present, otherwise it returns a new connection. A connection being present in the requestContext essentially means there is a request wide database transaction happening.
  • startRequestTransaction gets a fresh db connection, starts a transaction and stores it in requestContext for all subsequent database functions to consume.
  • doThingA and doThingB are functions that hit the database. They're unaware of whether they're in a transaction or not, that's decided for them further up and they just work with whatever connection is returned by getDBConn
import { createNamespace } from 'cls-hooked'
export const requestContext = createNamespace('request-context')

const app = express()

app.use(function requestContextMiddleware(req, res, next) {
  requestContext.run(() => next())
})
app.use(function setAuthenticatedUser(req, res, next) {
  // if request has auth header
  requestContext.set('authenticated-user', {})
})

const router = express.Router()

router.get('/some-mutation', async () => {
  try {
    await startRequestTransaction()

    await doThingA()
    await doThingB()

    await commitRequestTransaction()
  } catch (error) {
    await rollbackRequestTransaction()
  }
})

async function startRequestTransaction() {
  const dbConn = await getDBConn()
  await dbConn.startTransaction()
  requestContext.set('request-transaction', dbConn)
}
async function commitRequestTransaction() {
  const dbConn = requestContext.get('request-transaction')
  await dbConn.commit()
  requestContext.set('request-transaction', null)
}
async function rollbackRequestTransaction() {
  const dbConn = requestContext.get('request-transaction')
  await dbConn.rollback()
  requestContext.set('request-transaction', null)
}

async function getDBConn() {
  return (
    requestContext.get('request-transaction') || getNewDBConnFromSomewhere()
  )
}

async function doThingA() {
  const dbConn = await getDBConn()
  await dbConn.query('...')
  await dbConn.insert('...')
}

async function doThingB() {
  const dbConn = await getDBConn()
  await dbConn.query('...')
  await dbConn.insert('...')
}

Is this supposed to work? I also see runPromise mentioned in threads and issues but it's not mentioned in the docs at all. Is it better to use runPromise when you have lots of async function consuming the context downstream?

I can see there are some issues of similar nature still open but not sure if they're resolved. Does this approach just not work when there's async code involved?

Any help would be appreciated as this is happening in a prod app for us, we went all in on cls-hooked as it allowed us to have very simple domain logic code, didn't anticipate running into something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant