-
Notifications
You must be signed in to change notification settings - Fork 2k
feat onProxyReqWs async handler #1407
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1407 +/- ##
==========================================
+ Coverage 92.35% 92.47% +0.11%
==========================================
Files 6 6
Lines 314 319 +5
==========================================
+ Hits 290 295 +5
Misses 24 24
Continue to review full report at Codecov.
|
All tests have passed, can we please merge the PR? thanks |
any development on this PR? |
Would love to see this merged ! |
I've created a patch for this PR as well as one for the typescript types package:
diff --git a/node_modules/http-proxy/lib/http-proxy/passes/ws-incoming.js b/node_modules/http-proxy/lib/http-proxy/passes/ws-incoming.js
index 270f23f..809ec20 100644
--- a/node_modules/http-proxy/lib/http-proxy/passes/ws-incoming.js
+++ b/node_modules/http-proxy/lib/http-proxy/passes/ws-incoming.js
@@ -76,7 +76,7 @@ module.exports = {
*
* @api private
*/
- stream : function stream(req, socket, options, head, server, clb) {
+ stream : async function stream(req, socket, options, head, server, clb) {
var createHttpHeader = function(line, headers) {
return Object.keys(headers).reduce(function (head, key) {
@@ -104,9 +104,7 @@ module.exports = {
common.setupOutgoing(options.ssl || {}, options, req)
);
- // Enable developers to modify the proxyReq before headers are sent
- if (server) { server.emit('proxyReqWs', proxyReq, req, socket, options, head); }
-
+ // It is important to register the event listeners of proxyReq before the first await, so we don't have small period of time in which we lose emitted events
// Error Handler
proxyReq.on('error', onOutgoingError);
proxyReq.on('response', function (res) {
@@ -148,7 +146,23 @@ module.exports = {
server.emit('proxySocket', proxySocket); //DEPRECATED.
});
- return proxyReq.end(); // XXX: CHECK IF THIS IS THIS CORRECT
+
+ // Enable developers to modify the proxyReq before headers are sent
+ if (server) {
+ // Provides a way for the event handler to communicate back to the emitter when it finishes its async handling
+ let asyncHandler
+ const asyncContext = (callback) => {
+ asyncHandler = callback
+ }
+
+ server.emit('proxyReqWs', proxyReq, req, socket, options, head, asyncContext);
+
+ if (asyncHandler) {
+ await asyncHandler()
+ }
+ }
+
+ proxyReq.end();
function onOutgoingError(err) {
if (clb) {
diff --git a/node_modules/@types/http-proxy/index.d.ts b/node_modules/@types/http-proxy/index.d.ts
index ca805df..b674ff8 100755
--- a/node_modules/@types/http-proxy/index.d.ts
+++ b/node_modules/@types/http-proxy/index.d.ts
@@ -208,6 +208,7 @@ declare namespace Server {
socket: net.Socket,
options: ServerOptions,
head: any,
+ asyncContext: (cb: Promise) => void
) => void;
type EconnresetCallback<TError = Error, TIncomingMessage = http.IncomingMessage, TServerResponse = http.ServerResponse> = (
err: TError, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1407 +/- ##
==========================================
+ Coverage 92.35% 92.50% +0.14%
==========================================
Files 6 6
Lines 314 320 +6
==========================================
+ Hits 290 296 +6
Misses 24 24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Following issues #490 and #1403, currently the library does not support async handler for the
proxyReqWs
event for modifying the request before proxying it forward.This suggested PR solved the problem, without breaking backward compatibility, by introducing a new argument
asyncContext
which is used as follows: