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

Add ability to have custom 405 handler #759

Closed
wants to merge 4 commits into from

Conversation

meztez
Copy link
Collaborator

@meztez meztez commented Feb 5, 2021

Fixes #757

I could not explain it better than #757

This is crude, I would still prefer to have a more global approach to custom error handling

PR task list:

  • Update NEWS
  • Add tests
  • Update documentation with devtools::document()

NEWS.md Outdated Show resolved Hide resolved
@schloerke schloerke changed the title brute adding custom 405 Adding ability to have custom 405 handler Feb 5, 2021
@schloerke schloerke added this to the v1.1.0 milestone Feb 5, 2021
Co-authored-by: Barret Schloerke <barret@rstudio.com>
@schloerke schloerke changed the title Adding ability to have custom 405 handler Add ability to have custom 405 handler Feb 5, 2021
#' pr$set405Handler(function(req, res) {cat(req$PATH_INFO)})
#' }
set405Handler = function(fun){
private$unallowedMethodHandler <- fun
Copy link
Contributor

@cpsievert cpsievert Feb 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set404Handler should maybe do the same (to avoid errors that aren't caught until runtime)?

Suggested change
private$unallowedMethodHandler <- fun
stopifnot(is.function(fun))
args <- names(formals(fun))
if (!all(c("req", "res") %in% args)) {
stop("fun must have formal arguments for req and res")
}
private$unallowedMethodHandler <- fun

Copy link
Collaborator Author

@meztez meztez Feb 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple other plumber functions that take a function as input and are executed at runtime with potential failure. This might be material for another PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meztez Would you like / have time to do a PR for the other method parameter checks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually, I still have the regular job thing to do 😄

@meztez
Copy link
Collaborator Author

meztez commented Feb 9, 2021

Scrath this PR, I'm thinking long term we should have a better way to handler errors.

Something like serializers and parsers, a collection of error handler. This PR is more a bandaid than a real solution.

@meztez meztez closed this Feb 9, 2021
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.

HTTP 405 response bodies/headers cannot be customised
3 participants