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

Support all PUT operations, error later #118

Merged
merged 1 commit into from Sep 3, 2020

Conversation

RubenVerborgh
Copy link
Member

@RubenVerborgh RubenVerborgh commented Sep 2, 2020

When executing a bodyless PUT request, I would get:

UnsupportedHttpError: No handler supports the given input: [This handler only supports DELETE operations., This handler only supports GET operations., This handler only supports PATCH operations., This handler only supports POST operations., PUT operations require a body.].
    at CompositeAsyncHandler.findHandler (/Users/ruben/Documents/UGent/Solid/solid-community-server/src/util/CompositeAsyncHandler.js:77:15)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Clearly, the PUT handler is the (only) right one. The fact that it errors is because the request is invalid, and the PUT handler is the only one who knows that.

So the right error is “no body specified”, all the rest is noise. This PR brings it to:

UnsupportedHttpError: PUT operations require a body.

Apart from fixing this case, I also want to draw attention to the generic mechanism. canHandle answers the question: “is this the right actor to be handling this?’, not: “will handling succeed?”. In this case, the PUT actor can handle; it's just that the correct handling is to error. This means two things in particular:

  • canHandle methods should only check for applicability, not avoid any other errors via preconditions
  • calls to handleSafe are not guaranteed to not error; only to have that error thrown by the appropriate actor

I have created #119 to ensure this is applied consistently.

@joachimvh joachimvh merged commit 385e1a4 into master Sep 3, 2020
@joachimvh joachimvh deleted the fix/put-body-requirement branch September 3, 2020 07:02
@woutermont woutermont mentioned this pull request Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants