Try and catch around the JSON parse

[Dayjo]

This simply wraps the JSON parsing of the request in a try catch so we can throw an error.

It might be useful for us to throw an object containing the error and the body of the request so that the problem can be identified.

The other option would be to follow what you've done in constructObj and use new Error and return that, that way we can write a test for it. Do those get through to the .catch((err) ... on the api call?

@pat310 Any preference on the throwing vs returning new Error?

[coveralls]

Coverage decreased (-2.6%) to 91.176% when pulling b4f82a5937b1f1a9eb9a93b73d75eeb58da4890f on pallant:catch-json-error into f97dc4c8a39fa6fe3e34f4635d928bd875ff0b5f on pat310:master.

[pat310]

@Dayjo Yes they get through to the catch block on the api call but it's because in api.js it rejects the promise if the object is an Error object. You could move the parsing into a separate function so we could test for it...

[coveralls]

Coverage decreased (-3.2%) to 90.511% when pulling 20155acb6b22309a1bd2c1d6f6929e3c904b9093 on pallant:catch-json-error into f97dc4c8a39fa6fe3e34f4635d928bd875ff0b5f on pat310:master.

[Dayjo]

destroys more of the coverage

I assume it's fine to add a property to an Error. I doesn't seem to have any negative effects as far as I can see.

Any suggestion how I should test this? The way I was testing this was to edit the endpoint but as it's not a parameter not sure how to go about doing a test for it. Can probably test this once the Multiple Keywords is merged in as if you supply more than 5 terms, google 404s (though perhaps it should internally catch that error rather than doing the request and failing).

[pat310]

The best way to test it would probably be to move it to a separate function and then intentionally send it malformed JSON. Then you can check that the error response is as expected.

Hopefully I will be able to get your PR's merged in today.

[Dayjo]

I've updated this with tests.

Let me know if it could be done better! :)

[coveralls]

Coverage decreased (-1.8%) to 91.971% when pulling 94e7c0ced66d6c79cb994ab996ab23512b5ec630 on pallant:catch-json-error into f97dc4c8a39fa6fe3e34f4635d928bd875ff0b5f on pat310:master.

[Dayjo]

Aw god damnit even more coverage fail haha. Anyways, it's home time!

[pat310]

@Dayjo Looks good! Have some merge conflicts to resolve now, but when that's done I can approve the PR. Once this PR is merged in I'll release a new npm version.

[Dayjo]

Ugh, git is being a git about the conflicts.

[pat310]

@Dayjo Hmm, I'm going to .gitignore the build files - that will stop all these merge conflicts.

[Dayjo]

@pat310 yeah good idea! Why the hell it's decided to conflict that addition in utilities.js I have no idea.

[pat310]

@Dayjo Delete your lib folder, pull master and merge it into your branch. Then try to resolve any remaining merge conflicts (shouldn't be much). Let me know if you have problems.

[Dayjo]

Ok so I've totally gone and screwed this one up haha. I clearly have an awful workflow for dealing with forks.

[coveralls]

Coverage increased (+0.1%) to 95.139% when pulling ad8b8bd90c83edfceea7a822a1279124b8e0b2db on pallant:catch-json-error into 32dc7b5573601a7f4d2c95edb745304e205b3d5a on pat310:master.

[pat310]

@Dayjo I resolved the merge conflict, can you just correct these last few codeclimate issues?

[coveralls]

Coverage increased (+0.1%) to 95.139% when pulling ad8b8bd90c83edfceea7a822a1279124b8e0b2db on pallant:catch-json-error into 32dc7b5573601a7f4d2c95edb745304e205b3d5a on pat310:master.

[coveralls]

Coverage increased (+2.8%) to 97.753% when pulling 5359286209b839bf41dc68e25d081aab9bc2838d on pallant:catch-json-error into 32dc7b5573601a7f4d2c95edb745304e205b3d5a on pat310:master.

[coveralls]

Coverage increased (+2.8%) to 97.753% when pulling a316a9b3868a794da89117cb53f7a043a2edcbf9 on pallant:catch-json-error into 32dc7b5573601a7f4d2c95edb745304e205b3d5a on pat310:master.

[coveralls]

Coverage increased (+2.8%) to 97.753% when pulling a316a9b3868a794da89117cb53f7a043a2edcbf9 on pallant:catch-json-error into 32dc7b5573601a7f4d2c95edb745304e205b3d5a on pat310:master.

[Dayjo]

Phew!

[pat310]

@Dayjo Good job!!

[pat310]

@Dayjo Just published version 4.1.0 with the multiple keywords and JSON parse catch. Thanks!

[Dayjo]

Whoop! 🎉

Nice one, thanks for the assistance. Feels good to finally be able to contribute to some open source stuff while still at work!