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

Handled 404,500,422 Errors #58

Merged
merged 5 commits into from
Nov 8, 2016

Conversation

AfolabiOlaoluwa
Copy link
Collaborator

@AfolabiOlaoluwa AfolabiOlaoluwa commented Nov 5, 2016

@luizvarela @SundayAdefila. Review this.

See graphical representation below:

error fixed

Pushed handling 404,500, etc right to development.rb
Pushed configuration for handling 404,500, etc right to development.rb

def not_found
if env["REQUEST_PATH"] =~ /^\/api/
render :json => {:error => "not-found"}.to_json, :status => 404
Copy link
Contributor

@luizvarela luizvarela Nov 8, 2016

Choose a reason for hiding this comment

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

You can use the new hash syntax here: json: {error: 'not-found'}.to_json, status: 404

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lolll. @luizvarela right.

Pushed configuration for handling 404,500, etc right to development.rb

def not_found
if env["REQUEST_PATH"] =~ /^\/api/
render :json => {:error => "not-found"}.to_json, :status => 404
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a simple rescue_from will solve this problem to handle errors on the api side.

Copy link
Collaborator Author

@AfolabiOlaoluwa AfolabiOlaoluwa Nov 8, 2016

Choose a reason for hiding this comment

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

@luizvarela But talk of rescue_from, hope this wont rescue something else. Often am advised not to use it. Can you refactor this and well secured enough and let's see whats up?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can send a pull request later using the rescue_from approach. For now, it looks good

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So you agree we Merge?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure @AfolabiOlaoluwa, let's do this... 👍 🎉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay. I refactored to new hash syntax

edited application_controller.rb to use new hash syntax
@x6iae x6iae merged commit 901ac33 into devcenter-square:develop Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants