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

[CodeClimate] Added missing try-catch block #1613

Merged
merged 2 commits into from
Apr 1, 2018

Conversation

PyvesB
Copy link
Member

@PyvesB PyvesB commented Mar 27, 2018

This pull request should resolve #1607. I believe we were missing a try/catch block for the inner API request. It seems that were are kind of duplicating the same try/catch block twice for this service, but I'm unsure how to handle that in a better way. 😉

@shields-ci
Copy link

shields-ci commented Mar 27, 2018

Messages
📖

✨ Thanks for your contribution to Shields, @PyvesB!

Generated by 🚫 dangerJS

@paulmelnikow
Copy link
Member

Would it be worth adding a test with some incorrectly structured JSON?

@PyvesB
Copy link
Member Author

PyvesB commented Mar 28, 2018

@paulmelnikow I have added two tests, one for incorrectly structured JSON in the outer query and one for the inner query. I had some struggle with the icedfrisby-nock, and opened an issue there.

@PyvesB PyvesB added the service-badge Accepted and actionable changes, features, and bugs label Mar 28, 2018
sendBadge(format, badgeData);
} catch(e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the inner request call throws after its callback parameter returns, it leads to us writing the response twice, but I can't think of a neat solution apart from counting throws.

I guess it is pretty edge-case, huh? ☺

@espadrine espadrine merged commit dd35739 into badges:master Apr 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing try/catch in codeclimate
4 participants