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

Fall through still sets status code when handleError: false #222

Closed
troykelly opened this issue Jun 7, 2018 · 3 comments
Closed

Fall through still sets status code when handleError: false #222

troykelly opened this issue Jun 7, 2018 · 3 comments

Comments

@troykelly
Copy link

When config is set as handleError: false ecstatic is still setting the status code to 403 or 404 if there is no index or static file.
I would have expected that it would have left the status alone as express.static does.
Is there a way to truly not handle errors?

@jfhbrook jfhbrook changed the title Fall through still sets status code Fall through still sets status code when handleError: false Jun 7, 2018
@jfhbrook
Copy link
Owner

jfhbrook commented Jun 7, 2018

Sounds like a bug. If you feel like investigating, ecstatic's source code isn't actually all that big and only a little hairy. If you send me a pull request I will merge.

@troykelly
Copy link
Author

@jfhbrook It looks like it will be a fairly fundamental change.
Status is set during processing and passed on if handleError is false - It looks like it should be ignored throughout status-handlers - but that would be a major refactor.
I think this isn't so much a bug, it's just "expected behaviour"
My expectation of "handleError" was different to design.
The application does in fact not handle the error, but it does generate an error state.

@jfhbrook
Copy link
Owner

jfhbrook commented Jun 7, 2018

Hmmm

Yeah OK, I think I misunderstood the issue, it's been a minute since I've looked at this. It's coming back to me, though.

I think this issue is captured by #154 and you're right, it's a pretty major refactor. Loosely related is #183 , which would add restify support. My thinking here is to refactor to have a non-middleware API and then maybe a middleware-based adapter? But you're right, this is a bigger issue than I thought.

I'm gonna close this issue just cause I think #154 captures it pretty well, but if you disagree we can talk.

@jfhbrook jfhbrook closed this as completed Jun 7, 2018
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

No branches or pull requests

2 participants