v1.2: Migrate to KoaJS#344
Open
confused-Techie wants to merge 21 commits into
Open
Conversation
This setup is entirely borrowed from `confused-Techie/koa-test`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR migrates away from ExpressJS to KoaJS, another HTTP server library created by the same team, with a different structure.
I know it might seem rash to switch the library we use entirely, but let's dive into why I think this move makes the most sense.
What Doesn't work now?
Currently, I've tried to as best as possible use an OpenAPI-like specification to declare the backend, meaning our documentation is (as much as possible) our literal code. But there's been huge oversights with this, such as the actual logic of parameters being entirely separate from their OpenAPI schema, or our header handling being a complete afterthought all-together, that I had to make work with messy unintuitive
preLogicandpostLogichooks. Or, arguably the worst oversight, keeping thereqandresobjects away from thelogiccall itself, or even the underlying function calls that drive the mainlogiccall.All of this to mean that the actual server's conformance to our specification is nearly an afterthought, with plenty of places to deviate, and even more places where when we want a request to have direct access to the
responseobject, it has to be shoe-horned in. One easy place to find this difficult behavior is in error response handling, which goes through about three levels of obfuscation when leaving the mainlogiccall of an endpoint and reaching the user, making it difficult to refine further. I've accounted for this largely with testing the entire API call itself, but in the cases where we want to change it, it isn't the simplest or easiest task.What will be very difficult in the near future?
But, forward thinking here, there's plenty of things I really want us to be able to do here, that may be downright impossible with our current setup.
reqheaders we can't determine if a client can support this, and without easy access to thereswe can't change what we return, and lastly, with obfuscated error handling we don't have good insight to what a user gets during an error to even try.Server-TimingSpecification: I've wishlisted this for a while, but implementing and logging this everywhere means we can investigate and discover where our users have long waits and what takes longer than it should without them ever telling us, but again is impossible without better flow control of the entirerescycle, or having easy access to theresobject to create the headers needed, or having easy per-request write access on the context to make a global timer.Preferheader to return different pagination styles ImplementserviceVersionsemver aware filtering #310 : The results of this PR make it obvious that to add post-filtering steps we need to migrate the pagination style we use, which if we want to stay backwards compatible, can likely only be done while being able to respond to thereqobject's header's to understand the preference of yet to be created support in our various clients.What I've tried instead?
Over in #297 I tried a huge rewrite of this codebase, where we completely rewrote the OpenAPI-like specification we use, created a per-request context object, and a bunch of other features covered in #299 #296 and #298 but even then, I felt like with so much code churn it wasn't as good as it could be, and still exposed blindspots that would come to be a problem in the future.
Really that PR shows a solution for right now, but doesn't feel extensible enough to support us in the future without more disruptive, major changes.
What we should do? (What's in this PR?)
Instead, I'd suggest that we do exactly what this PR does, which superseeds all the PRs listed in "What I've tried instead". Which is migrate to KoaJS and at the same time redefine our OpenAPI-like Specification, and context object.
The difference between doing those other two tasks here or in the other PR, is that migrating already causes huge changes, might as well do everything all at once.
If we migrate to KoaJS, which is a middleware focused library, means we get a couple of advantages immediately.
What this PR does?
resandreqobjects, meaning everyone will have access to it as needed.You'll notice that all the items in "What will be very difficult in the near future" and "What doesn't work now" essentially all focus on a lack of access to the
req&resobjects, and difficulty in controlling the request flow, or modifying the context object. Which KoaJS's structure seems very likely to fix, and is exactly why I think this is the solution here.What this PR Accomplishes?
Below will be a list of items from the above lists that this PR should accomplish in order to be considered successful:
Server-TimingSpecificationPreferHeaders to modify Pagination Handlingcontextobjectreqandresobjects