-2

I have a block of code that calls a JS function (NodeJS). The function it calls contains a Promise chain. Here is the code that calls the function:

'use strict'

const request = require('request')

try {
  const data = search('javascript')
  console.log('query complete')
  console.log(data)
} catch(err) {
  console.log(err)
} finally {
  console.log('all done')
}

function search(query) {
  searchByString(query).then( data => {
    console.log('query complete')
    //console.log(JSON.stringify(data, null, 2))
    return data
  }).catch( err => {
    console.log('ERROR')
    console.log(err)
    throw new Error(err.message)
  })
}

function searchByString(query) {
  return new Promise( (resolve, reject) => {
    const url = `https://www.googleapis.com/books/v1/volumes?maxResults=40&fields=items(id,volumeInfo(title))&q=${query}`
    request.get(url, (err, res, body) => {
      if (err) {
        reject(Error('failed to make API call'))
      }
      const data = JSON.parse(body)
      resolve(data)
    })
  })
}

When I run the code, the console displays query complete followed by the search results.

Then I get an error: TypeError: google.searchByString(...).then(...).error is not a function which doesn't make sense! Why does this error get triggered?

Mark Tyers
  • 2,961
  • 4
  • 29
  • 52
  • 2
    Unless you're using some Promise library, you want `.catch()` and not `.error()`. – Madara's Ghost Nov 12 '16 at 20:55
  • 1
    Definitely `.catch` but also, that try/catch that you have will never work because that is inside a synchronous function and your promise logic is async. – loganfsmyth Nov 12 '16 at 21:01
  • Thanks for spotting the typo Madara. I've consolidated all the code into one script for clarity. Now I get `query complete` but no data. I can see that the data is returned in the wrong order for the catch to work. – Mark Tyers Nov 12 '16 at 21:03
  • your search is async, but you try using it sync. Have a look at http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call to understand async concepts better – Johannes Merz Nov 12 '16 at 22:12

2 Answers2

0

well, you're assigning the return value to data, but you're not returning the result of the promise!

Here's a working solution for you where the only thing i did was a) return the promise in search and b) get the data from the resolved promise.

you can see this code and play with it at: https://runkit.com/arthur/5827b17b8795960014335852

'use strict'

const request = require('request')

try {
  search('javascript')
    .then(
      data => console.log('and the data', data),
      error => console.error('uh - oh', error)
    );
    console.log('the query isn\'t done yet!');
} catch(err) {
  console.error(err);
} finally {
  console.log('all done, or is it?')
}

function search(query) {
  return searchByString(query).then( data => {
    console.log('query complete')
    //console.log(JSON.stringify(data, null, 2))
    return data
  }).catch( err => {
    console.log('ERROR')
    console.log(err)
    throw new Error(err.message)
  })
}

function searchByString(query) {
  return new Promise( (resolve, reject) => {
    const url = `https://www.googleapis.com/books/v1/volumes?maxResults=40&fields=items(id,volumeInfo(title))&q=${query}`
    request.get(url, (err, res, body) => {
      if (err) {
        reject(Error('failed to make API call'))
      }
      const data = JSON.parse(body)
      resolve(data)
    })
  })
}

Below is how I'd go about it from scratch. I think the biggest improvement I have here is using the node url lib. Always a good thing to do so string escaping is handled for you. In you're case, if the user were to hand in a string like "don't", it'd break :).

require('request');
// request promise is a popular, well tested lib wrapping request in a promise
const request = require('request-promise');
// i like to use url handling libs, they do good things like escape string input.
const url = require('url');

class Search {
    constructor(query) {
        this.query = query;
    }

    fetch() {
        const endpoint = url.parse('https://www.googleapis.com/books/v1/volumes');

        const options = {
            maxResults: 40,
            fields: 'items(id,volumeInfo(title))',
            q: this.query
        }

        endpoint.query = options;

        const callUrl = url.format(endpoint);
        return request.get(callUrl).then(result => JSON.parse(result));
    }
}

const search = new Search('javascript');
search.fetch()
    .then(result => console.log(result))
    .catch(e => console.error('catch:', e));

and here's the working code: https://runkit.com/arthur/5827d5428b24ed0014dcc537

Arthur Cinader
  • 1,547
  • 1
  • 12
  • 22
0

Now I get query complete but no data.

That is because you log data before the request.get() operation has completed. This block:

 try {
  const data = search('javascript')
  console.log('query complete')
  console.log(data)
} catch(err) {
  console.log(err)
} finally {
  console.log('all done')
}

executes synchronously and therefore before the code following it. You have to change it to:

data = search('javascript')

.then(function(data){
    console.log('query complete');
    console.log(data);
    console.log('all done')
})
.catch(function(err){
    console.log(err);
})
rabbitco
  • 2,790
  • 3
  • 16
  • 38