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

middleware body needs reworked #148

Closed
katanacrimson opened this issue Apr 26, 2019 · 0 comments
Closed

middleware body needs reworked #148

katanacrimson opened this issue Apr 26, 2019 · 0 comments

Comments

@katanacrimson
Copy link
Contributor

katanacrimson commented Apr 26, 2019

Horribly vague title, I know, but here's the gist of what my gut says is going wrong.

Let's look at the closure lambda.

We have some incredibly vague logical flow happening due to the logical branching point once we get within the parsedMethods check. We consistently set the bodyPromise according to the kind of request we're working with, but the contents of that bodyPromise may differ slightly along with how we handle it. We also have the unhandled "else" case where the request type is disabled or doesn't match criteria, which falls through to where we handle the bodyPromise itself. IMO, we're doing ourselves a disservice in understandable code by trying to avoid the minor amount of duplication in handling assignment to ctx.request.body by separating the setting of bodyPromise and the bodyPromise.then. We're doing some unique handling inside of the callback...so we might as well have unique handlers to begin with.

Effectively, this else case should be a no-op, but isn't. We could optimize here and make code clearer by letting the application flow exit the middleware faster.

Also, if we're okay with introducing async/await, we can clean up our rough try/catch pattern here that's slightly duplicated in order to provide some better error handling.

We should also either consolidate the isMultipart function or dissolve it; it's oddly repetitive and serves no purpose.

Of note, this sort of cleanup should allow us to rectify #112, and #115.

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

1 participant