Better error handling - Githubissues

zxol / airbnbapi

Unofficial airbnb.com REST API wrapper for node.js

MIT License

218 stars 52 forks source link

Better error handling #21

Open Technohacker opened 5 years ago

Technohacker commented 5 years ago

For issue #5 Functions now throw Error objects instead of returning nulls

Potential issues:

Errors from request-promise and airbnbapi are both given back to the caller. More code may be necessary to give a unified interface
This would very likely be a breaking API change

This PR isn't intended to be merged right away until reviewed for more issues

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 50

99 of 119 (83.19%) changed or added relevant lines in 2 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+6.9%) to 77.358%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
build/main.js	98	118	83.05%
<!--	Total:	99	119	83.19%	-->

Files with Coverage Reduction	New Missed Lines	%
build/log.js	1	47.83%
<!--	Total:	1		-->

Totals
Change from base Build 49:	6.9%
Covered Lines:	217
Relevant Lines:	278

💛 - Coveralls

zxol commented 5 years ago

That's brilliant! Thank you!

It looks like you may have edited the build files instead of the pre-babelised source code in the src directory. Totally understandable as I have neglected to write a contribution guide. My apologies.

How would you like to proceed?

Technohacker commented 5 years ago

I've updated both (source then babelize), it's just that I didn't notice a gitignore for the build directory so I thought you kept both updated together :sweat_smile:

The only commit that affects the build files would be the last one so the rest can be used instead :D

zxol commented 5 years ago

Ahh sorry, Understood.

zxol commented 5 years ago

Did you write a script to help you refactor the code? I'm curious about your approach to the problem.

Technohacker commented 5 years ago

To be honest I was changing lines manually :rofl: I used Atom so for a majority of the lines I used multi-cursor editing :)

zxol commented 5 years ago

Nice :)

zxol commented 5 years ago

I'll review this carefully and try to make some additions as well, prior to merging. Perhaps we should notify the users about an upcoming API surface change, too.

Technohacker commented 5 years ago

Yes that's a good idea. I'll keep building with the API and add more endpoints if necessary so all of it can be made into one release :)

zxol commented 5 years ago

Sounds good. I'm really glad to have a collaborator on the project.