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

Does this library work with await? #132

Open
RichardJECooke opened this issue Jul 1, 2016 · 24 comments
Open

Does this library work with await? #132

RichardJECooke opened this issue Jul 1, 2016 · 24 comments
Labels

Comments

@RichardJECooke
Copy link

RichardJECooke commented Jul 1, 2016

Can I just call const result = await rp(options) or do I have to call rp(options).then(...) to actually trigger the request to start?

Some libraries, like superagent, won't work without the latter syntax.

It would be wonderful if you added this info to your home page readme.

@analog-nico
Copy link
Member

👍 for being progressive!

const result = await rp(options) will be enough. The request is started right away.

Since I am not using ES7 yet, could you provide a small code snippet that you tested out and I can paste into the README?

@RichardJECooke
Copy link
Author

RichardJECooke commented Jul 6, 2016

Code like this seems to work fine:

  const request = require('request-promise');

  async function preparePayment(cents: number): Promise<boolean> {
    const options = {
      method: `POST`
      ,json: true
      ,uri: `https://pay.org/v1/checkouts`
      ,body: { 'amount': `99` }
    };
    try {
      const response = await request(options);
      return Promise.resolve(response.id == 5);
    }
    catch (error) {
      Promise.reject(error);
    }
  }

@analog-nico
Copy link
Member

Thanks a lot for the snippet @RichardJECooke !

I will keep this issue open until I added the snippet to the README.

@hildjj
Copy link
Contributor

hildjj commented Jul 6, 2016

@RichardJECooke, I have a few questions and suggestions for your code:

  • Is this typescript?
  • your catch doesn't call return
  • I don't think async functions need to return a promise; they do that automatically
  • you didn't use cents
  • using template strings for things that aren't templates probably isn't a best practice yet

so, something like this:

const request = require('request-promise');

async function preparePayment(cents) {
  const options = {
    method: 'POST'
    ,json: true
    ,uri: 'https://pay.org/v1/checkouts'
    ,body: { 'amount': cents }
  };
  try {
    const response = await request(options);
    return (response.id == 5);
  }
  catch (error) {
    return false;
  }
}

@RichardJECooke
Copy link
Author

RichardJECooke commented Jul 6, 2016

  • It's Typescript. For Javascript, just remove the : number.
  • async does return a promise automatically, I've just been writing them manually to avoid any weird errors
  • Template strings allow you to write multiline strings and have no disadvantages that I know of.

Here's some slightly better (almost real) code I'm using in production (without the static types):

async preparePayment(cents) {  
    const options = {
      method: 'POST'
      ,headers: { 'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8' }
      ,json: true
      ,uri: `http://example.com/v1/checkouts`
      ,form: {
        'authentication.userId': `username`,
        'amount': cents.toString()
      }
    };
    try {
      const response = await request(options);
      return Promise.resolve(response);
    }
    catch (error) {
      return Promise.reject(error);
    }
  }

@cspotcode
Copy link

@RichardJECooke I think you're right about template strings; no real disadvantage to using them, except maybe that you have to avoid putting ${ in the string.

You mentioned that superagent won't work unless you explicitly call .then(). I thought the await keyword would always do that for you. Is there a specific example of superagent causing trouble? I ask because I use async functions all the time, so I'd like to learn about any possible corner cases.

@RichardJECooke
Copy link
Author

You'll need to use superagent-promise. But I had issues setting headers in GET calls with it so I switched to this. I've been very happy with request-promise. Has worked perfectly so far.

@KpjComp
Copy link

KpjComp commented Jul 21, 2016

 try {
      const response = await request(options);
      return Promise.resolve(response);
    }
    catch (error) {
      return Promise.reject(error);
    }

Isn't that just the same as doing ->

const response = await request(options);
return response;

Not sure why you would wrap a try / except only to return a promise that the async function is going to do for you anyway..

@RichardJECooke
Copy link
Author

RichardJECooke commented Jul 21, 2016

No, that's not the same. It is the same as this though:

return request(options);

The problem with your example is that you are missing the catch block (which acts the same as .catch() for normal promises).

Without it you'll have uncaught unhandled silent exceptions.


As for why, because this question is about documentation. And my example clearly shows how to use await with promises using try and catch.

@KpjComp
Copy link

KpjComp commented Jul 21, 2016

Without it you'll have uncaught unhandled silent exceptions.

My understanding, if you mark a function with async, it will always return a promise, unhanded exceptions should then return a rejected promise.

Also the return request(options) is not assigning to the response object, my assumption was that was meant to be used in some way between the 2 lines before returning the async function, obviously in this example it's redundant, and the use of await is pointless.

@KpjComp
Copy link

KpjComp commented Jul 21, 2016

I think @RichardJECooke origianl code can also just be ->

const request = require('request-promise');

async function preparePayment(cents: number): Promise<boolean> {
  return (
    await request({
      method: `POST`
      ,json: true
      ,uri: `https://pay.org/v1/checkouts`
      ,body: { 'amount': `99` }
    })
  ) === 5;
}

Here the await isn't pointless as the return is getting transformed into a boolean return.
Note, no messy try / excepts. And exceptions should get handled without problems.
Not only is the code shorter, but to me seems easier to follow.

@cspotcode
Copy link

@KpjComp is actually correct on this one. The catch isn't necessary because async functions, by nature, will catch any thrown errors and turn them into promise rejections.

All 3 of the following are identical:

async function doStuff() {
    // Option 1
    const response = await request(options);
    return response;
    // Option 2
    return await request(options);
    // Option 3
    return request(options);
}

@EvanCarroll
Copy link
Contributor

This should be closed. I'm currently using the library now with async await on v7.0.0-test2016100609987d242b. It is working.

@buzai
Copy link

buzai commented Jun 22, 2017

@cspotcode
hello, i test it ,but // Option 1 only return a promise,i want to return data.

@cspotcode
Copy link

@buzai it's an async function so it always returns a promise, no matter what. That's how async functions are required to work. So everything is working correctly.

@buzai
Copy link

buzai commented Jun 23, 2017

@cspotcode yes,thank u , just i know it .

@analog-nico
Copy link
Member

@buzai Where you able to find a solution? If not please post your code snippet and info about which environment you are using. I’ll take a look.

@buzai
Copy link

buzai commented Jul 13, 2017

@analog-nico i just use await to get some info
finally i use egg.js to struct my api , in egg project , it awalys use yield, so i can use it any way.

@alex-shamshurin
Copy link

alex-shamshurin commented Nov 28, 2017

The problem is correctly distinguish errors from http request and from other code:

async makeRequest = (options) {
  try {
  // some code which can cause an error
  const body = rp(options)
  // some code with body (or response) which can throw an exception
  // for example XML parsing to JSON
  } catch (err) {
    // is err a http. error wiht non 200 status
   // is err another technical error related with request
   // maybe smth else, XML parsing error
  }
}

@analog-nico
Copy link
Member

@alex-shamshurin const errors = require('request-promise/errors') defines a few error types that you can use to differentiate certain case. For example you can use if (err instanceof errors.StatusCodeError) { /* Handle non-2xx responses here */ }.

@bupthebroker
Copy link

bupthebroker commented Dec 7, 2017

How do you guys use await with request? I get the following error:
SyntaxError: await is only valid in async function

const resp = await request(o);
return resp;

I need to block until i have the response from request.

@RichardJECooke
Copy link
Author

See my code snippet in the 2nd comment. You can use 'await' only when it's inside a function prefixed with 'async'. This tells Javascript/Typescript to treat 'await' specially (not just as another variable name)

@froid1911
Copy link

lol, nice thread. :-D I guess this can closed

@jbistis
Copy link

jbistis commented Apr 1, 2019

Richard, from a service I am running:
getVesselLocation(id: string) {
return this.fns.httpsCallable('getVesselLocation')({ query: id });
}

And the function has:
const options = {
uri: finalUrl,
json: true,
auth: {
username: config.xxxxxxx.apiUsername,
password: config.xxxxxxx.apiSecret
}
}

...and

try {
    const result = await rp(options);
    console.log('result',result);
    return result;
} catch (error) {
    console.log('error',error);
    return Promise.reject(error);
    // return null;    }

I get the error I get the error {"status":"error","message":"Not authorized to access this endpoint"}

but instead of getting the error message sent back so I can show it to the user in the browser, I get a console error...
ERROR Error: Uncaught (in promise): Error: INTERNAL
Error: INTERNAL
at new HttpsErrorImpl (index.cjs.js:58)
at _errorForResponse (index.cjs.js:153)

Any help would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests