Change errror code to 404 if no app is found #2343
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently an error code of 500 is returned when no app is found for a
route, but this changes it to a 404 which seems more appropriate. See
this issue for more context:
unbit#1935
This partially reverts the previous commit. Now it will return 500 if the no-default-app setting is unset. This makes sense since in that case there should always be some app to choose from. But if no-default-app is set, then it returns a 404 if one is not found.
|
I made a slight change to the PR so that a 404 is only returned if no-default-app is set. I think my original change was a bit too broad. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently an error code of 500 is returned when no app is found for a route, but this changes it to a 404 which seems more appropriate. See this issue for more context:
#1935
Honestly I'm not sure if this change is really possible since maybe the currently functionality is required for compatibility. I tried running tests, but I had some issues. At the very least this appears to fix the problems for me so I figured I would open up a PR to solicit feedback.