Skip to content

Conversation

@tj
Copy link
Contributor

@tj tj commented Sep 1, 2011

it's pretty trivial to trigger a next(err) for almost any middleware, by-design they should be handled in some unified fashion, however the one in this lib just throws. You could also emit an error event with the req/res to conditionally allow users to customize the handling if necessary

@mikeal
Copy link

mikeal commented Sep 1, 2011

+1

@mikeal
Copy link

mikeal commented Sep 1, 2011

one concern i have is that this 500 is indistinguishable from a 500 returned by the applications behind the proxy.

we could add a header 'x-node-proxy-error', or change the status code to one that is proxy specific, the problem is that there isn't one that fits perfectly.

10.5.3 502 Bad Gateway

The server, while acting as a gateway or proxy, received an invalid response 
from the upstream server it accessed in attempting to fulfill the request.

10.5.5 504 Gateway Timeout

The server, while acting as a gateway or proxy, did not receive a 
timely response from the upstream server specified by the URI 
(e.g. HTTP, FTP, LDAP) or some other auxiliary server (e.g. DNS) 
it needed to access in attempting to complete the request.

@tj
Copy link
Contributor Author

tj commented Sep 1, 2011

true true, not sure what convention is there

@mikeal
Copy link

mikeal commented Sep 1, 2011

bah, fuck it, let's just change the text body to "Internal Proxy Server Error" :)

@tj
Copy link
Contributor Author

tj commented Sep 1, 2011

beats nothing. looks like squid adds custom 6xx range status codes


Broken Server Software
600
Squid: header parsing error
601
Squid: header size overflow detected while parsing
601
roundcube: software configuration error
603
roundcube: invalid authorization

@mikeal
Copy link

mikeal commented Sep 1, 2011

node.js should define all kinds of new status codes that all begin with 31337 ;)

@indexzero
Copy link
Member

Thanks. I'll push this in tonight.

@indexzero
Copy link
Member

Sorry, been busy with NodeConf SummerCamp. Will push this out when I'm back in NYC tonight.

@tj
Copy link
Contributor Author

tj commented Sep 8, 2011

sounds good thanks man!

@indexzero
Copy link
Member

Cherry-picked in 07c8d2e. Will be published in 0.6.7

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