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

Behavior change for bot.recall('not yet stored key')? #13

Open
jpjpjp opened this issue May 15, 2017 · 1 comment
Open

Behavior change for bot.recall('not yet stored key')? #13

jpjpjp opened this issue May 15, 2017 · 1 comment
Assignees
Milestone

Comments

@jpjpjp
Copy link
Contributor

jpjpjp commented May 15, 2017

Did the update of the new storage/redis module change the overall behavior of storage?

This is sample code from Hotel California:

var htc = bot.recall('htc');
 // if htc has not been initialized to bot memory...
  if(!htc) {
    htc = bot.store('htc', {});
    // store default value
    htc.enabled = false;
  }

I have similar code to this in my bot, but since doing an npm update, I'm now getting a non-null value returned when the key has never been set. Rather it returns an unhandled error object. I also see:

Potentially unhandled rejection [1] Error: bot.recall() could not find the value referenced by id/key
at Object.recall (/Users/jshipher/Dropbox/dev/node/tropo-jira-notification-bot/node_modules/node-flint/storage/memory.js:60:32)

I can handle this in my code, but wondered if this behavior is planned/expected.

@nmarus
Copy link
Member

nmarus commented May 18, 2017

Unexpected, but yes, you are correct. The bot.store/recall/forget code was refactored a few commits back along with the memory storage module to return or reject promises instead of returning a null value. The above would look something like this. I'll update the readme htc example.

bot.recall('htc')
  .then(function(htc) {
    // if htc is found
    // [ ... ]
  })
  .catch(function(err) {
    // if htc is not found
    htc = bot.store('htc', {});
    // store default value
    htc.enabled = false;
  });

Of note, the redis storage module still uses the old behavior. I had removed it during a previous commit, it is back now but should be refactored to be interchangeable with the memory storage module.

@nmarus nmarus self-assigned this May 18, 2017
@nmarus nmarus closed this as completed Sep 19, 2017
@nmarus nmarus reopened this Sep 19, 2017
@nmarus nmarus added this to the Flint v5 milestone Sep 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants