Skip to content

Improve pinboard error handling #213

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 15, 2018
Merged

Improve pinboard error handling #213

merged 4 commits into from
Feb 15, 2018

Conversation

@gbirke
Copy link
Contributor

@gbirke gbirke commented Feb 9, 2018 •
edited
Loading

Don't crash node RED server on failed HTTP status codes or JSON errors in return body.

Some code cleanup.
Added network tests.

gbirke added 2 commits February 9, 2018 23:34
@gbirke
Improve code style
77049a2 
Remove trailing whitespaces
Add semicolons
@gbirke
Improve pinboard error handling
27ec0cd 
Use HTTPS.

Check the HTTP status code to avoid trying to parse empty JSON messages.
Catch JSON parsing errors.
@jsf-clabot
Copy link

jsf-clabot commented Feb 9, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage decreased (-0.2%) to 71.566% when pulling 27ec0cd on gbirke:robust-pinboard into 545fd70 on node-red:master.

1 similar comment
@coveralls
Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage decreased (-0.2%) to 71.566% when pulling 27ec0cd on gbirke:robust-pinboard into 545fd70 on node-red:master.

@coveralls
Copy link

coveralls commented Feb 9, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.5%) to 72.269% when pulling e1887a2 on gbirke:robust-pinboard into 545fd70 on node-red:master.

@gbirke
Speed up pinboard tests
6591f37 
Instead of using timeouts, use sinon.js to stub the error functions.
This also improves code quality as the test code no longer needs to know
the logging internals.
@gbirke
Test pinboard network requests
e1887a2 
use nock to intercept pinboard network requests.
Change http.get to http.request to play nice with our 4-year old version
of nock.
If the server (or network mock) sends no HTTP status message, improvise.
Show user-friendly, translatable status messages, log raw errors.
@dceejay
Copy link
Member

dceejay commented Feb 15, 2018

looks good - are you done with updating test etc ? OK to merge ?

@gbirke
Copy link
Contributor Author

gbirke commented Feb 15, 2018

I'm done with this improvement, thank your for asking.

@dceejay dceejay merged commit 49b051d into node-red:master Feb 15, 2018
@dceejay
Copy link
Member

dceejay commented Feb 15, 2018

Thanks for this.
(especially for adding the tests :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gbirke @jsf-clabot @coveralls @dceejay