Remove http-network-proxy and use http-proxy-middleware directly #147
Conversation
| return proxy; | ||
| this.server = this.app | ||
| // This lets us see the json body of the request in the proxy middleware | ||
| .use(express.json()) |
There was a problem hiding this comment.
What happens if the response isn't json? Do we have other middleware to handle those? Does it also still proxy json requests?
There was a problem hiding this comment.
If it is not a json body then we do not have access to it. Theoretically someone could still get the body by adding observers on the req but just about all apis that you would want to inspect are json. In the future we could add more but json was all that we had before as well and never used anything else so we'll see if someone requests it.
There was a problem hiding this comment.
So not even xml or plain text would funnel through?
There was a problem hiding this comment.
It isn't captured. If someone wanted to they could still read it off of req with some work but I would say that's not the common use case. I would say xml and form body would probably be the biggest things to add but all we use is json currently. If someone sees a need then we can look into it then.
| * Below method is triggered by the middleware that helps us to overwrite the path. | ||
| */ | ||
| private rewritePath(path, req) { | ||
| const urlParts = url.parse(req.originalUrl.replace('/;', '')); |
There was a problem hiding this comment.
Is this semicolon the standard way of writing proxy URLs? Like is this how it works for charles as well? If not, it might be good to align on whatever "normal" proxy convention is out there (i don't know, haven't used them much).
There was a problem hiding this comment.
Yes correct that is how it is done in Charles as well.
| const urlParts = url.parse(req.originalUrl.replace('/;', '')); | ||
| path = urlParts.path || path; | ||
|
|
||
| // If Charles proxy we want to use the original url instead |
There was a problem hiding this comment.
Does this only apply to Charles proxy, or would it work for others? Might be good to clarify why we know there's already a proxy in the pipeline and can skip.
There was a problem hiding this comment.
It could be applied to other applications as well if they follow the same format as Chris details here https://github.com/chrisdp/Roku-Charles-example but Charles is the main one people use
Co-authored-by: Bronley Plumb <[email protected]>
…n NetworkProxy.start(). Add removeCallback function in processRequest and processResponse args to simplify cleanup
…av/roku-test-automation into remove_http-network-proxy_depedency
No description provided.