Added request.catch

[Revadike]

Avoid uncaught exceptions, because request is a promise.

[Revadike]

https://github.com/codemanki/cloudscraper/issues/194

[TravisBuddy]

Travis tests have failed

Hey @Revadike, Please read the following log in order to understand the failure reason. It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 10

View build log

npm test

``` > cloudscraper@3.9.1 test /home/travis/build/codemanki/cloudscraper > npm run lint && nyc --reporter=html --reporter=text mocha > cloudscraper@3.9.1 lint /home/travis/build/codemanki/cloudscraper > eslint --ext .json --ext .js . /home/travis/build/codemanki/cloudscraper/index.js 143:17 error Expected error to be handled handle-callback-err 144:5 error Unnecessary return statement no-useless-return ✖ 2 problems (2 errors, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option. npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! cloudscraper@3.9.1 lint: `eslint --ext .json --ext .js .` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the cloudscraper@3.9.1 lint script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above. npm ERR! A complete log of this run can be found in: npm ERR! /home/travis/.npm/_logs/2019-04-13T15_41_25_505Z-debug.log npm ERR! Test failed. See above for more details. ```

Node.js: 8

View build log

npm test

``` /home/travis/build/codemanki/cloudscraper/index.js 143:17 error Expected error to be handled handle-callback-err 144:5 error Unnecessary return statement no-useless-return ✖ 2 problems (2 errors, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option. npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! cloudscraper@3.9.1 lint: `eslint --ext .json --ext .js .` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the cloudscraper@3.9.1 lint script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above. npm ERR! A complete log of this run can be found in: npm ERR! /home/travis/.npm/_logs/2019-04-13T15_41_36_343Z-debug.log npm ERR! Test failed. See above for more details. ```

Node.js: 6

View build log

npm test

``` Done. Your build exited with 1. ```

TravisBuddy Request Identifier: a1a45dc0-5e02-11e9-9212-e1acf2d44b97

[Revadike]

I agree it's bad to dismiss the caught error as I did, but I'm not sure how to handle it in your environment. I've loved to be "coached" on how to do it properly.

Anyway, it does fix the uncaught error message whenever I got an error using cloudscraper with callbacks.

[ghost]

If a callback is provided, I agree that the error makes little sense other than to encourage users to use request as the requester.

First, you'll want to consider how this change will affect the user base. If they're using request as the requester, it won't affect them at all. If they're using request-promise with a callback but still expecting a promise to either be resolved or rejected (We do this in almost every test), you'll be breaking their code.

Thus it's a breaking change and requires updating the documentation, a major version bump, etc. Not to mention rewriting all of the tests. Assuming none of that deters you from wanting to introduce this change, continue reading...

The first thing you're going to want to do is check to see whether the user supplied a callback. You can determine that by checking to see if options.callback is a function in the performRequest function. Next, determine whether or not the requester is request-promise. If it's not request-promise, the catch method won't exist and there is no need to do anything at all. I'm sure there was something else that was bugging me about this change but I can't recall what it was, maybe it was nothing... Let me know if you need anymore assistance.

Generally, the addition of a condition will cause the code to branch which means test code coverage will decrease if you don't add a test to cover that branch of code. You'll want to become familiar with the unit tests and ask yourself how that change might affect those tests. It's likely that some tests will need to be modified. If you miss something, that's what reviews are for so no worries.

Oh and make sure that the repository maintainers support your idea before putting the time in, otherwise you might find that your PR isn't accepted despite your effort.

[ghost]

@Revadike Did you decide whether or not you still want to work on this?

[codemanki]

@Revadike if you ever decide to get back to this PR, please feel free to reopen it.

[ghost]

@codemanki That's a nice sentiment but he can't reopen this PR because you closed it. Had he been the one to close it then he'd be able to reopen it but not otherwise. @Revadike could always just open a new PR with the changes and reference this one if he wants to refer to the comment history. Simply asking this to be reopened is an option as well.

[codemanki]

@pro-src @Revadike sorry guys, I wasn't aware of this fact. Tho I could reopen it if @Revadike pings me here

[ghost]

I completely agree with the decision to close it for now. I just wanted to let you know that Github isn't that liberal. :laughing:

[Revadike]

Nah, in fact, I deleted the whole branch.