node-persist: Concurrent `setItem` and `getItem` can lead into an unahdled `does not look like a valid storage file`

Concureent setItem and getItem can lead into an unahdled does not look like a valid storage file:

parse error:  {} for:
     Error: [node-persist][readFile] .node-persist/storage/37b51d194a7513e45b56f6524f2d51f2 does not look like a validstorage file!
      at fs.readFile (node_modules/node-persist/src/local-storage.js:278:66)
      at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:525:3)

The code used to reproduce create that error was:

const localdb = require('node-persist')
const storage = localdb.create({ ttl: true, logging: true })

async function test2 () {
  await storage.setItem('bar', { 'foo': 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.' })
  await storage.getItem('bar')
  await storage.setItem('bar', { 'foo': 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.' })
  await storage.getItem('bar')
  await storage.setItem('bar', { 'foo': 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.' })
  await storage.getItem('bar')
}

async function test3 () {
  await storage.getItem('bar')
  await storage.getItem('bar')
  await storage.getItem('bar')
  await storage.getItem('bar')
  await storage.getItem('bar')
}

async function main () {
  await storage.init()

  try {
    await Promise.all([
      test2(),
      test3(),
      test2(),
      test3(),
      test2()
    ])
  } catch (err) {
    console.error(err)
  }
}

main().catch(err => {
  console.error(err)
})

The problem cannot be realiable be reproduces.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 23

Most upvoted comments

When storage.init (storage being node-persist) encounters a corrupt file it throws this error.

I tried using try/catch and .catch() callbacks to handle this error gracefully, but neither work because the error is not bubbling up to where I can handle it.

For those looking for a drop-in replacement for this library, I’ve switched to https://www.npmjs.com/package/node-localstorage and it seems to be working fine. The api is basically similar with getItem() and setItem()

robbie-cahill on Jul 16, 2020

I think the better approach would be to serialise read and writes based on the queue. One approach as mentioned in my SO answer(see the gist) above is to chain promises. I think we can enhance it to simply chain promises per key rather than whole universe of read and write through the library.

Please see the basic idea here : https://gist.github.com/biswanaths/b8999c9e4e5a9e979dcde061074528f6

I believe whatever solution to this problem should live in library. The end user does not need to know and work around whether the library is storing kv per file or using a single file.

biswanaths on Oct 4, 2018

I’ve also hit this problem, but in my case, I’m not performing asynchronous get/sets but a series of ~10 sets asynchronously using Promise.all(array.map([function that creates promises])). The library hits on the same file name a number of times and ends up writing n values to the same file, thus ending up with a corrupt file.

Given that asynchronicity is commonplace in nodejs, it would be a lot better if this was handled by the library using file locks.

DragonEmbers on Sep 4, 2019