jeremydaly / lambda-api

Lightweight web framework for your serverless applications
https://serverless-api.com
MIT License
1.42k stars 125 forks source link

Middleware does not run for '/' route when using base paths #172

Open Sleavely opened 3 years ago

Sleavely commented 3 years ago

When using a base path, the / route doesn't seem to run middleware properly.

I wrote a failing test case using CORS as an example to illustrate:

it('Middleware runs for root path when basepathed', async function() {
  const testApi = require('../index')({
    base: 'base-path'
  })

  testApi.use((req, res, next) => {
    res.cors()
    next()
  })
  testApi.use((err, req, res, next) => {
    res.cors()
    next()
  })
  testApi.get('/', async () => {
    return { status: 'ok' }
  })

  let _event = Object.assign({},event,{ path: '/base-path/' }) // even got a trailing slash and everything :(
  let result = await testApi.run(_event,{})

  expect(result.multiValueHeaders).to.have.property('access-control-allow-origin')
})

The route itself runs and returns { status: 'ok' } but the CORS headers are missing. If the registered route is changed it works as expected:

// ...
 testApi.get('/potato', async () => {
    return { status: 'ok' }
  })
  let _event = Object.assign({},event,{ path: '/base-path/potato' })
// ... this works.

Interestingly, the error middleware runs fine regardless of the route. If you remove the route / and change the event path to something like '/base-path' you'll get a 404 (or 405, don't recall) but the CORS headers will be there

jmjoyceiv commented 7 months ago

Can confirm this is still occurring as of January 2024 / 1.0.3. A fix would be fantastic. Explicitly targeting / in addition to the global call works, e.g.:

api.use('/', apiCommon.corsMiddleware);
api.use(apiCommon.corsMiddleware);