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.
Horribly vague title, I know, but here's the gist of what my gut says is going wrong.
Let's look at the
closurelambda.We have some incredibly vague logical flow happening due to the logical branching point once we get within the
parsedMethodscheck. 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 toctx.request.bodyby separating the setting of bodyPromise and thebodyPromise.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
isMultipartfunction 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.