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

Rescue addressable uri error with proper error message #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rescue addressable uri error with proper error message #1

wants to merge 1 commit into from

Conversation

jgrevich
Copy link
Member

I noticed the default input for pages#index, 'http://', throws a 500 error (see:
http://discovery.my.usa.gov/pages?url=http%3A%2F%2F). I added a rescue clause to
render a more informative error message using json. I also included a test against
such malformed URIs.

@jgrevich
Copy link
Member Author

Just to clarify, the default input "http://" is used here: http://apidocs.presidentialinnovationfellows.org/mygov-discovery

@greggersh
Copy link
Contributor

Thanks Justin! Good catch, this looks great. Can I ask you a favor? Can you resubmit this pull request, but squish your two commits down into a single commit (do a 'git rebase -i master' on your branch). And if there's a way to have the pull request come in to a branch on this repo, can you do that as well? Email me at greg AT shelrick . com if you have any questions. Thanks!

If you haven't considered doing so already, you should also apply to Project MyUSA at http://www.whitehouse.gov/innovationfellows/myusa. ;) Applications are due tomorrow.

I noticed the default input for pages#index, 'http://', throws a 500 error (see:
http://discovery.my.usa.gov/pages?url=http%3A%2F%2F). I added a rescue clause to
render a more informative error message using json.

I also added a spec to test for malformed url.  I'd like to test against
additional error prone URLs but this should suffice for now.
@jgrevich
Copy link
Member Author

Hi Greg, I'm glad to be of service. I squashed the commits from the original pull request as you described: #2c9e940. I don't believe there is a way for me to create a new branch in your repo without being a part of the GSA-OCSIT group (I'll keep my fingers crossed regarding my PIF application :), thanks for letting me know!).

Perhaps we should look into a workflow with dev + feature branches similar to the Diaspora project (https://github.com/diaspora/diaspora/wiki/Git-Workflow , adapted from here: http://nvie.com/posts/a-successful-git-branching-model/).

I'm no git expert but I believe you can manually (e.g. outside github) grab this branch by adding me as a remote repository. You can then checkout the branch from this pull request and push to github. I can also point the pull request to a different branch if you have one in mind. Let me know if there is anything I can do to help.

Cheers!

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.

2 participants