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

Catch error for malformed URL #20

Closed
wants to merge 2 commits into from
Closed

Catch error for malformed URL #20

wants to merge 2 commits into from

Conversation

th507
Copy link

@th507 th507 commented May 18, 2015

Proposed fix for #19 .

This also adds minor compatibility changes for latest io.js.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 97.12% when pulling 5a2212b on th507:master into c9304e6 on koajs:master.

@jonathanong
Copy link
Member

it's supposed to fail. they are malformed URLs.

@th507
Copy link
Author

th507 commented May 20, 2015

@jonathanong Theoretically, yes. But when using trie-router in production environment, we would like to redirect all unmatched URLs to a nice error page.

IMHO, router (along with Koa) should allow developer to redirect error to a specific page.
The 'problem' with trie-router is that it proactively throws a "Malformed URL" error and returns a 404 page, which breaks the conventional way of handling error with Koa's middleware.

@jonathanong
Copy link
Member

i'd rather throw a very specific error with an err.code that you could try/catch upstream. this allows devs to do whatever they wish with the error, including re-throwing as a 404

@tejasmanohar
Copy link
Member

👍 for specific error available for try/catch upstream

@tejasmanohar
Copy link
Member

This PR except with a specific err.code is available here -> #22 @jonathanong
and the test script changes are here -> #23

@haoxins haoxins closed this Nov 22, 2015
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.

None yet

5 participants