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

Unable to use data outside of promise? #71

Closed
jessevdp opened this issue Nov 29, 2016 · 10 comments
Closed

Unable to use data outside of promise? #71

jessevdp opened this issue Nov 29, 2016 · 10 comments

Comments

@jessevdp
Copy link

Hi, I'm creating a wrapper module around yours to make it easier to use for my purposes.
However I'm having trouble while getting an item from storage.

Here's my code.

write: function (key, data, callback) {
    var storageObject = {
      // Add the data we want to store to the storage Object.
      data: data,
      // Add the time of creation to the storage Object.
      created: new Date().getTime()
    }

    /* Save the item to our node-persist cache. */
    storage.setItem(key, storageObject, callback);

  },
  read: function (key) {
    storage.getItem(key)
      .then(function (value) {
        console.log('promise: '+value.data);
        return value.data;
      });
  },

Those functions are both on module.exports

Then somewhere else in the file I do this:

module.exports.write('user', 'xxxx', function (err) {
  if (err) {
    console.log('ERROR');
  }
  console.log('user: '+module.exports.read('user'))
});

When I run this file in node I get the following
screen shot 2016-11-29 at 20 29 45

My question

What am I doing wrong here? I do return the value of the Object I'm getting at the same time of logging it, however It's undefined when logging it? This is weird in my opinion.

@akhoury
Copy link
Collaborator

akhoury commented Nov 29, 2016

well, both setItem and getItem are asynchronous and return a promise, you will not be able to log the value from your read function immediately - also, your read function is not returning anything, and if it would, it can only return the promise.

 read: function (key) {
    return storage.getItem(key) // <-- notice I added a return here, so your read() function would return the PROMISE but not the VALUE 
      .then(function (value) { // <--- anonymous function
        console.log('promise: '+value.data);
        return value.data; // <-- this return is for the anonymous function, not your read function
      });
  },

You have 2 options, either you read/write asynchronously, at this point you can't immediately use the values, you must wait for the promises to resolve, and if you are using promises, you dont need to use the callbacks

write: function (key, data) { // i removed the callback
    var storageObject = {
      // Add the data we want to store to the storage Object.
      data: data,
      // Add the time of creation to the storage Object.
      created: new Date().getTime()
    },
    /* Save the item to our node-persist cache. */
    return storage.setItem(key, storageObject); // <-- I also added a return here and remove the callback

  },
  read: function (key) {
    return storage.getItem(key) // <-- added return here to return the promise
      .then(function (value) {
        console.log('promise: '+value.data);
        return value.data; // <-- this doesn't do much except making your resolve data be value.data instead of value
      });
  },

then to use it

write('abc', 'foo')
  .then(function() {
    return read('abc');
  })
  .then(function(data) {
   console.log('user', data);
  });

OR

you make everything synchronous, and

write: function (key, data) { // i removed the callback
    var storageObject = {
      // Add the data we want to store to the storage Object.
      data: data,
      // Add the time of creation to the storage Object.
      created: new Date().getTime()
    },
    /* Save the item to our node-persist cache. */
    return storage.setItemSync(key, storageObject); // <-- I also added a return here and remove the callback, but made the function `setItemSync`

  },
  read: function (key) {
    var value = storage.getItemSync(key); // <-- changed it to use the Sync version to get the value immediately.
     return value && value.data; // since you were trying to return just data
    // the `value &&` just makes sure there is a truthy value before accessing `value.data`
      });
  },

@akhoury
Copy link
Collaborator

akhoury commented Nov 29, 2016

what are you trying to do if you dont mind me asking? one thing you MUST know is that you CANNOT wrap an asynchronous "function" or "code block" and make it synchronous, and promises are asynchronous

@jessevdp
Copy link
Author

Thanks a lot for the quick reply.
As I'm reading this I realize that this is all pretty straightforward actually. Everything sounds really logical.

Guess coding 5 hours in a row isn't good for anyone 😂.

Cheers!

@akhoury
Copy link
Collaborator

akhoury commented Nov 29, 2016

hehe good luck!

@akhoury akhoury closed this as completed Nov 29, 2016
@jessevdp
Copy link
Author

I'm trying to create a cache that keeps the data stored even when the TTL has ran out.

This is to make sure that even if the database fails to get the data we need, we still have outdated cached data available.

The database is our schools place for storing grades and whatnot... And it errors all the time when sending a request to it.

My app uses that database and does stuff with the data.

So just adding a timestamp to the data and then checking to see if the TTL has ran out. If it has we make a call to the database. If that fails we use outdated data.

Also, I don't want to pollute my main file with all that code... So I decided to write a module...

Make sense?

@akhoury
Copy link
Collaborator

akhoury commented Nov 29, 2016

a cache that keeps the data stored even when the TTL has ran out.

Which TTL? your school's database has a TTL for its data and you're trying to cache it "locally"?

@jessevdp
Copy link
Author

My app generates advice for the students based on their grades. To get the grades I make a call to the database of our school and in that call I specify the user so I'll get their data.

However I found that making too many calls to the schools database was bad practice and on top of that the schools database seemed to error a lot of the times.

So I decided to cache the data locally after a request for that specfic user has been send. However the data in the database might change... So we need to give the data a TTL locally in our cache....

But to make sure we'd always have some data available to work with after the first call I don't want to delete the data after the TTL has ran out.. I rather want to say it's not valid anymore.

So we can still use it if the database starts acting up again.

Clearer now?

@akhoury
Copy link
Collaborator

akhoury commented Nov 29, 2016

Yup- ok here's the thing, you don't need to implement your own TTL mechanism, this module has TTL support, just enable the ttl option when you initialize the storage module.

storage.initSync({
    dir:'relative/path/to/persist',
    ttl: false, // ttl*, can be true for 24h default or a number in MILLISECONDS
});

The other thing I suggest you do is to throttle your requests to your school's database, using something like https://github.com/SGrondin/bottleneck or https://github.com/jhurliman/node-rate-limiter - i used these a lot when i am dealing with third party APIs where they limit the amount of requests I can make per second.

Finally, if you are using an HTTP request to hit your school's database and you don't want to cache them to disk, but just cache things in memory, you can use this quick module (instead) I made for that purpose, the difference is that the latter only caches in RAM, so if you shutdown your node process, you loose the cache, it's more like a short-term cache thing and watch out of filling up your memory, I suggest having a short TTL https://github.com/akhoury/request-promise-cache

@jessevdp
Copy link
Author

Hey, thanks for the tip on the throttling. I'll definitely look into that!

I know your software has a TTL option.. However I'm pretty sure this makes sure that after the time to live has ran out the data will be deleted.

I rather want to make something that's very much like a TTL but not quite the same.
I call it a TTL but I'd rather name it something different entirely.

The timestamp is designed to make sure that the cached data is still valid, or recent enough to use.
*note the part where I described using the data even after the TTL has run out.

I'm probably gonna make a method on the wrapper module like isRecent(key)

As for saving the data to the drive. Thats pretty much the goal here. My server might shut down and I want to absolutely make sure that I almost never ever have to send a request to the database. But just sometimes when the data isn't recent anymore.

Even when the server shuts down or crashes. The data should still be there!

@akhoury
Copy link
Collaborator

akhoury commented Nov 30, 2016

I see. have fun !

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

2 participants