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

Recover for panics in function Handler for HTTP mode #27

Closed
s8sg opened this issue Jul 16, 2019 · 6 comments
Closed

Recover for panics in function Handler for HTTP mode #27

s8sg opened this issue Jul 16, 2019 · 6 comments
Labels
wontfix This will not be worked on

Comments

@s8sg
Copy link

s8sg commented Jul 16, 2019

In current code we are calling the http function handler as

result, resultErr := function.Handle(req)

In case there is a panic inside the handler the handler server process will exit, failing the future requests for the same.

Optionally it can be handled with a recover as:

defer func() {
       if recoverLog := recover(); recoverLog != nil {
               log.Print(recoverLog)
               w.WriteHeader(http.StatusInternalServerError)
       }
}()
result, resultErr := function.Handle(req)

In this case the handler server keep running

@LucasRoesler
Copy link
Member

I like this, is it something you would be willing to also contribute?

@s8sg
Copy link
Author

s8sg commented Jul 16, 2019

@LucasRoesler Sure

@alexellis
Copy link
Member

Hi, thanks for your suggestion.

This is actually by design. I think that it should fail and that the container should be restarted.

If someone has a specific need for a catch-all, then the PR can be forked. I might change my mind in the future, but this is where I am on it at the moment.

Alex

@s8sg
Copy link
Author

s8sg commented Jul 16, 2019

That make sense. One can handle the panic inside the request handler itself and return a response accordingly. It gives more flexibilities to determine whether to recover or not

@alexellis
Copy link
Member

I appreciate the suggestion. Please keep on helping us to make OpenFaaS better 👍

@alexellis
Copy link
Member

/add label: wontfix

@derek derek bot added the wontfix This will not be worked on label Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants