Promise version?

[orecus]

Love this library, but I was just wondering if there have been any thoughts about moving to promises for this?

[impronunciable]

That'd be cool. It should be easy to wrap 🤔

[orecus]

Yes, I thought about wrapping it myself, but thought I would ask here before I started looking at it if it where planned. I think it would be great instead of wrapping it myself. :)

My thought was to use it together with promise-queue to get some form of control on the amount of requests being sent trough the various calls, it would be great if it would follow the api-limits.

[impronunciable]

@orecus I think is a good idea. If you want to implement it go ahead and I can review it and help

[orecus]

@impronunciable I'll try to take a look at it in the coming days and see what I can do. :)

Some alternatives for some form of rate limiting: https://github.com/JMPerez/promise-throttle and https://github.com/azproduction/promise-queue could work perhaps if you would wish to implement rate limiting in the plugin itself.

[orecus]

Sadly I havn't had time too look at the code to try to implement this nativly within MovieDB. I solved this by simply wrapping the calls with moviedb as promises and using https://github.com/JMPerez/promise-throttle, it does seem to work perfectly fine.

[impronunciable]

Great, is there any code I can take a look at?

[orecus]

Sure, here are a quick and dirty example from the code i have, it will be available in full once the application i'm writing is in a shape to be shared. :)

You call searchMovie and that will initiate a throttled API call, from my quick tests it seems to work fine.

const PromiseThrottle = require('promise-throttle');
const throttle        = new PromiseThrottle({ requestsPerSecond: 4, promiseImplementation: Promise });
const mdb             = require('moviedb')(tmdbAPIKey);

  function internal_searchMovie(searchData) {
    let query = internal_searchTerm(searchData);

    return new Promise(function (resolve, reject){
      mdb.searchMovie(query, function(error, searchResults){
        if (error) {
          return reject({error: error.response.body.errors[0], type: 'TMDB Error'});
        } else {
          return resolve(searchResults.results);
        }
      });
    });
  }

  function searchMovie(data, respond) {
    if (data.id != undefined) {
      respond(null, data);
    } else {
      throttle.add(internal_searchMovie.bind(this, data))
      .then((searchResults) => {
        respond(null, searchResults);
      })
      .catch((error) => {
        respond(null, {error: error});
      });
    }
  }

[staaky]

Here's a quick little wrapper that supports all API calls.

// moviedbp.js
var MovieDB = require('moviedb');
var endpoints = require('moviedb/lib/endpoints.json');
var limits = require('limits.js');

// rate limiting
var queue = limits();
queue.within(1000, 4);

function MovieDBP(api_key, base_url) {
  this.moviedb = MovieDB(api_key, base_url);
  return this;
}

Object.keys(endpoints.methods).forEach(function(method) {
  var met = endpoints.methods[method];
  Object.keys(met).forEach(function(m) {
    MovieDBP.prototype[method + m] = function(params) {
      var self = this;
      if (typeof params == "function") {
        params = {};
      }

      return new Promise(function(resolve, reject) {
        queue.push(function() {
          self.moviedb[method + m](params, function(err, res) {
            if (err) return reject(err);
            resolve(res);
          });
        });
      });
    };
  });
});

module.exports = function(api_key, base_url) {
  return new MovieDBP(api_key, base_url);
};

const mdbp = require('moviedbp')('your api key');

mdbp.searchMovie({ query: 'Alien' })
.then((res) => { console.log(res); })
.catch((err) => { console.log(err); });

Maybe an option to switch between callbacks and Promises would be nice? It breaks chaining but that's something you could document. Something like:

require('moviedb')({ api_key: 'your api key', promise: true });

The rate limiting in my snippet could be used for both.

[daviestar]

Promise wrap:

const _mdb = require('moviedb')('your-key')

const mdb = (m, q) => new Promise((res, rej) => {
  _mdb[m](q, (err, data) => err ? rej(err) : res(data))
})

mdb('searchMovie', {query: 'Alien'})
  .then(console.log)
  .catch(console.error)

With limits:

const _mdb = require('moviedb')('your-key')
const limits = require('limits.js')
const throttle = limits().within(1000, 3)

const mdb = (m, q) => new Promise((res, rej) => {
  throttle.push(() => _mdb[m](q, (err, data) => err ? rej(err) : res(data)))
})

mdb('searchMovie', {query: 'The Martian', year: 2015})
  .then(console.log)
  .catch(console.error)

[favna]

Through "testing" I see you ended up implementing the method as proposed by @daviestar but you haven't updated the readme yet, might I recommend doing that as well as closing the issue so it's clear to people that promises are available?

Also to add some code feedback to this general feedback, the promises method also allows for an async/await approach:

const moviedb = require('moviedb')(auth.TheMovieDBV3ApiKey);

async run(msg, args) {

    const tmdb = (m, q) => new Promise((res, rej) => {
            moviedb[m](q, (err, data) => err ? rej(err) : res(data)); // eslint-disable-line no-confusing-arrow
        }),
        tmdbres = await tmdb('searchMovie', {
            'query': args.name
        });

    if (tmdbres) {
        return console.log(tmdbres);
    }

    return console.log('error');
}

Edit: a full implementation of the code above can be seen here, which is for my Discord bot: https://gist.github.com/Favna/5951fe0760553e99c0db139794ee4228

[daviestar]

@Favna I don't think a promise implementation has made it to this repo yet, but as you've already done it's easy to wrap it.

When using async/await be careful to manually catch any promise rejection:

try {
  const tmdbres = await tmdb('searchMovie', {...})
  if (tmdbres) {
    return console.log(tmdbres)
  }
  return console.log('no response')
} catch (err) {
  return console.error(err)
}

Using the promise wrapper above is great for one-off searches but if you intend to loop through a big list of movies and grab metadata, you should take a look at the slightly more complicated implementation here: https://github.com/impronunciable/moviedb/pull/74#issuecomment-284155721

[mocanew]

@favna check out https://github.com/grantholle/moviedb-promise