hapijs / catbox

Multi-strategy object caching service
Other
494 stars 72 forks source link

Default memory caching is not promise safe #207

Closed holmok closed 5 years ago

holmok commented 5 years ago

When using require('make-promises-safe') the default memory caching kills the application. Here is some sample code...

// require('make-promises-safe')
const Hapi = require('hapi')

const server = Hapi.server({
  host: 'localhost',
  port: 8000
})

server.route({
  method: 'GET',
  path: '/',
  async handler (request, h) {
    const now = await request.server.methods.date()
    return `hello world it is ${now}`
  }
})

server.method(
  'date', 
  () => new Date().toISOString(), 
  { cache: { expiresIn: 2000, generateTimeout: 100 } }
)

async function start () {
  try {
    await server.start()
  } catch (err) {
    console.log(err)
    process.exit(1)
  }
  console.log('Server running at:', server.info.uri)
};

start()

This code works fine with // require('make-promises-safe') commented out, but as soon as you uncomment it and call to const now = await request.server.methods.date() in a handler the process ends and outputs this to the console.

resolve Promise { '2018-11-18T16:50:31.617Z' } undefined

devinivy commented 5 years ago

I tested this out and can confirm. It seems to be due to multipleResolves. Going to continue and take a closer look.

devinivy commented 5 years ago

The promise that is resolving twice is the Promise.race() found here. Related to a potential V8 quirk mentioned in this issue https://github.com/nodejs/node/issues/24321.

kanongil commented 5 years ago

Catbox is intentionally using multiple resolves (which has apparently been declared unsafe). I'm not sure what to do here.

kanongil commented 5 years ago

This is all rather quite silly. Now, if I want to make a deferred, where I intend to support multiple resolves / rejects, I need to keep track of the state of the promise (replicating the internal state).

devinivy commented 5 years ago

I believe this is resolved (😉) in make-promises-safe v4.

holmok commented 5 years ago

👍 thanks.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.