websocket proxy cannot upgrade websocket connection

Expected behavior
HPM should upgrade websocket requests always
Actual behavior
HPM websocket proxy cannot upgrade websocket connections sporadically
Setup

http-proxy-middleware: 0.17.1
server: tornado 4.4.1 - Tornado Github
webpack: 1.13.2
webpack-dev-server: 1.16.1

proxy middleware configuration
 devServer: {
      proxy: {
        '/hyperguard/websocket/*': {
          target: 'ws://localhost:8082',
        ws: true
        }
      }
    }

server mounting
def serve_forever(self):
    app = self.__make_app()
    self.__http_server = HTTPServer(app)
    self.__http_server._handle_connection = self._handle_connection
    self.__http_server.listen(self.__port, self.__host)
    app.ssl_enabled = self.__ssl_options is not None
    self.__ioloop.start()

Howto Reproduce
http stream request
GET /hyperguard/websocket HTTP/1.1
Host: localhost:8079
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Sec-WebSocket-Version: 13
origin: http://localhost:8079
Sec-WebSocket-Extensions: permessage-deflate
Sec-WebSocket-Key: yrOcNHCvvVuGgttYgv9nzA==
Connection: keep-alive, Upgrade
Pragma: no-cache
Cache-Control: no-cache
Upgrade: websocket

http stream response
getting this response only sometimes, if not the websocket connection never be established
HTTP/1.1 101 Switching Protocols
upgrade: websocket
connection: Upgrade
sec-websocket-accept: I3u4B7iakSJWk4yLd02hfn+enus=

that issue also occurs on different platforms (fedora 22, ubuntu 14.04, centos7)
restarting webpack-dev-server doesnt change anything

Thanks for reporting.
Can you provide info on the websocket client(s) you are using.
And the frequency of the upgrade requests?
_.debounce is used to solve an issue: #57. Maybe that explains the sporadic behaviour.
https://github.com/chimurai/http-proxy-middleware/blob/0cb6839ed2121f7dc8685f31a1e18b5069eeb092/lib/index.js#L15
Can you try to remove the debounce code and see if it solves the issue?
From:
var wsUpgradeDebounced  = _.debounce(handleUpgrade);

To:
var wsUpgradeDebounced  = handleUpgrade;

We are using the websocket client implementation of Chrome(53.0.2785.116 m (64-bit)) and Firefox (47.0). Frequency: I try to establish only one connection on initialisation. Nothing changed on removing the debounce code.

Just to confirm if the issue is related to HPM.
Did you try to connect to the server directly? (without the proxy)
If is it HPM related; It can be either an issue in HPM configuration or a bug in HPM.
Try adding the option:  changeOrigin: true
Tornado might be refusing the request, since Host value in the request is different from the Tornado's host.
 devServer: {
      proxy: {
        '/hyperguard/websocket/*': {
          target: 'ws://localhost:8082',
          changeOrigin: true,
          ws: true
        }
      }
    }

Direct connection
I think the issue is related to HPM, because if i request a websocket upgrade directly it always works as expected.
request (without HPM)
GET /hyperguard/websocket HTTP/1.1
Host: localhost:8082
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
Sec-WebSocket-Version: 13
Origin: https://localhost:8082
Sec-WebSocket-Extensions: permessage-deflate
Sec-WebSocket-Key: 7XjMl4KmXDJa/TWfb7vabQ==
Connection: keep-alive, Upgrade
Pragma: no-cache
Cache-Control: no-cache
Upgrade: websocket

immediatlely response (without HPM)
HTTP/1.1 101 Switching Protocols
Upgrade: websocket
Connection: Upgrade
Sec-WebSocket-Accept: iC7Eh/cX6YgchA7mJQ950DWmIiQ=

Workaround
I figured out a way to reproduce a workaround with tornado + HPM + webpack-dev-server.
If the problem occurs i just have to request the websocket proxy path via http e.g http://localhost:8079/hyperguard/websocket
response
HTTP/1.1 400 Bad Request
X-Powered-By: Express
Content-Length: 34
Date: Fri, 23 Sep 2016 09:13:38 GMT
Server: TornadoServer/4.4.1
Content-Type: text/html; charset=UTF-8
Connection: keep-alive

Can "Upgrade" only to "WebSocket".

If i go back to my working url e.g. http://localhost:8079 the websocket connection can be upgraded as expected.
Q&R

Adding changeOrigin option has no effect.
Tornado actually allows alternate origins.

Can you try setting your target with the http protocol:
target: 'http://localhost:8082'

Changing the protocol of the target option has no effect. I have to debug the issue in-depth. It cannot be excluded that the problem occures on tornados side.

Think I've exhausted the common configurations. :)
FYI:
HPM uses http-proxy to do the actual proxying. (https://github.com/nodejitsu/node-http-proxy)
Worth checking to see if you'll get the same issue when you're just using http-proxy.

It looks like I get this with webpack-dev-server 1.16.2 also. I get this on the server:
[HPM] GET /websocket -> ws://localhost:8080/websocket
[HPM] Upgrading to WebSocket

Chrome reports:
(index):37 WebSocket connection to 'ws://<IP_ADDRESS>:8888/websocket' failed: Connection closed before receiving a handshake response

I'm using a similar config:
proxy: {
	'/websocket': {
		ws: true,
		target: 'ws://localhost:8080/websocket',
		logLevel: 'debug'
	}
}

One thing I do see which hasn't been reported before is nothing happens if I perform raw websocket connection on a cold start, it just hangs. I only get the above output if I first attempt a basic GET to http://<IP_ADDRESS>:8888/websocket first which appears to warm-up HPM. I have tried connecting directly to
ws://localhost:8080/websocket and it does work, I've also tried changing ws:// to http:// to no avail.
The closest issue I've found in the http-proxy repo is nodejitsu/node-http-proxy#577. I've also raised this on StackOverflow.

@dansiviter, could you try to change your target to ws://localhost:8080? AFAIK the path /websocket already gets appended (unless you use the pathRewrite option).

Thanks, one step closer as that worked. However, I still need to warm up the connection with a GET before it'll connect. Any ideas?

debugged that problem weeks ago rudimentary, it seems that the http proxy middleware which is applied to the express server never be executed. maybe a side effect with sockjs which is used to handle the hot realoading websocket channel.

Same problem here. If the websocket upgrade is the very first request to the server then it just hangs. Happens only on chrome though (tested on 47.0.2526.73). Firefox works fine.
Has anyone solved this issue?

@dansiviter this SO answer might be helpful: http://stackoverflow.com/a/32943389/465887
I'm working on enabling websocket proxying in Create React App (https://github.com/facebookincubator/create-react-app/issues/1013) and it looks like I can manually watch for the "upgrade" request coming from the devServer, and then call HPM's "upgrade" function.
CRA is using HPM directly though, instead of the devServer.proxy config, so it might not apply to your use case, but this is the code I'm using to set it up (so far it seems to be working but I haven't tested thoroughly yet):
    // Pass the scope regex both to Express and to the middleware for proxying
    // of both HTTP and WebSockets to work without false positives.
    var hpm = httpProxyMiddleware(pathname => mayProxy.test(pathname), {
      target: proxy,
      logLevel: 'debug',
      onError: onProxyError(proxy),
      secure: false,
      changeOrigin: true,
      ws: true
    });
    devServer.use(mayProxy, hpm);

    devServer.listeningApp.on('upgrade', hpm.upgrade);

I had problems with the proxy websocket closing right after connecting. Found out that socket io was closing unhandled requests. If you are using socket io, the fix is to set the destroyUpgrade option to false:
var express = require('express');
var app = express();
var http = require('http').Server(app);
var proxy = require('http-proxy-middleware');
var io = require('socket.io')(http, {destroyUpgrade: false});

Also seeing this issue.  @mixxen's fix seems to have worked for me.

Apologies for commenting on an ancient ticket, but I managed to hit this issue (ws proxy only starts working after manually performing a GET call).
app.use(
    '/ws/live',
    createProxyMiddleware({
        target: process.env.API,
        ws: true, // enable websocket proxy
        changeOrigin: true,
        logLevel : 'debug'
    })
);

...
"http-proxy-middleware": "^1.0.6",
"react": "^17.0.1",
"react-scripts": "4.0.0",
....
I understand that no resolution was found, but does anyone know any workaround (better than manually doing a GET, I mean)?

Same here. AFAIU issue on the middleware happens if server is trying to send data socket which has already been closed, but I haven't dig it enough yet. I'll try to find more details, just wanted to underline that issue persists in some conditions.

Got stuck with this issue in my CRA app. I have changed my setupProxy.js to the following and It looks like it's working for me:
app.use(
    '/',
    createProxyMiddleware('/ws', {
      target: process.env.API,
      changeOrigin: true,
      secure: false,
      ws: true,
    })
  );

So that the request to React App itself works as a "first manual GET" for the WebSocket.

Got stuck with this issue in my CRA app. I have changed my setupProxy.js to the following and It looks like it's working for me:
app.use(
    '/',
    createProxyMiddleware('/ws', {
      target: process.env.API,
      changeOrigin: true,
      secure: false,
      ws: true,
    })
  );

So that the request to React App itself works as a "first manual GET" for the WebSocket.

I am not entirely sure how this works... but it does! Thank you for sharing this workaround!

@torrejonv from my understanding it works like lazy loading. Until you make a request to the route where proxy middleware is registered there will be no "upgrade" handler added to the HTTP server. With this approach, any request passes through the proxy middleware so that we have an upgrade handler registered from the very beginning.

@torrejonv from my understanding it works like lazy loading. Until you make a request to the route where proxy middleware is registered there will be no "upgrade" handler added to the HTTP server. With this ☝ approach, any request passes through the proxy middleware so that we have the "upgrade" handler registered from the very beginning.

Thank you for the explanation. Very ingenious indeed!


Question regarding the service layer and DDD

Hi Jon
In an [article by Martin Fowler] (http://www.martinfowler.com/bliki/AnemicDomainModel.html) he mentions the following: "This (service) layer is kept thin. It does not contain business rules or knowledge, but only coordinates tasks and delegates work to collaborations of domain objects in the next layer down."
In your [SampleWebApp] (https://github.com/JonPSmith/SampleMvcWebApp) which uses the GenericServices, the concrete classes and dto classes replicate some domain logic, like [Required] or [MinLength]. Is there any way to hold this centrally within the concrete classes and not duplicate this kind of logic.
What do you think about this?
Cheers
Jonathan

Hi @riffrack,
Good question. I agree with Martin Fowler, who I like a lot. GenericServices aims to be an adapter and facard with no business logic. However if we use DTOs, which hide some of the validation in the data classes, what do you do?
You could leave out the Validation from the DTO and rely on the validation in the data layer. Problem is you lose browser level validation, which would really hurt the user experience of live error feedback. While having validation in the Service Layer breaks the rules I think putting validation in the DTOs is acceptable. Validation is a bit like Security in that it is a cross-cutting problem.
I would also like to point out that MVC's answer to this is to put the validation in the Presentation layer with ViewModels, which I think is even worse that being in the ServiceLayer. With GenericServices I am trying to make the MVC layer just about presentation, although some business logic creeps in, especially in JavaScript, if you want to provide a first class user experience.


Add tests for IntegerExtensions: isPowerOfTwo and isPrime

We can discuss naming for the prime tests, I kind of went with the first thing that came to mind

isPowerOfTwo returns false for negative powers of two. Is this expected behavior?

Current coverage is 84.03% (diff: 100%)

Merging #20 into master will increase coverage by 2.39%

@@             master        #20   diff @@
==========================================
  Files            28         29     +1
  Lines           490        526    +36
  Methods           0          0
  Messages          0          0
  Branches          0          0
==========================================
+ Hits            400        442    +42
+ Misses           90         84     -6
  Partials          0          0

Powered by Codecov. Last update bb882ca...fde6bea

And sorry about houndci. It's on the list to add configuration for it, rather than accepting the default (which has some weird opinions).

Changes made. This PR won't pass until #21 is merged.

Also, we can resolve #2 after this.

If one of these tests fail, it's a travis integration hiccup. Also on deck for cleansing.
Looks great!

Awesome! :)
The test was failing because one of the test cases was on Int64.min, which #21 fixed.


Upload file to webdav using a stream

I use your lib for downloading files from a webdav server and works perfectly. I'm using it with the exposed stream that was later added here:
https://github.com/skazantsev/WebDavClient/pull/52
and using it like this:
https://github.com/skazantsev/WebDavClient/issues/31
I can also upload files, but not with progress report, since PutFile doesn't support streams the way GetRawFile() does. Is that something that is possible to add to the lib? Passing a stream to PutFile and then start writing to that stream so that the content is uploaded to the webdav server?

Hi @unconnected ,
I haven't tried this but I believe it's possible to get upload progress by passing an HttpClient instance with a custom message handler to the WebDavClient.
There's an example of such handler in one of old ASP.NET libs.
It would be nice to support progress of upload/download within the lib but I don't have near future plans to add it.
But I'd be happy to accept Pull Requests for this.

I haven't tried this but I believe it's possible to get upload progress by passing an HttpClient instance with a custom message handler to the WebDavClient.

Thanks for the heads up, I'll dive into this :P

I haven't tried this but I believe it's possible to get upload progress by passing an HttpClient instance with a custom message handler to the WebDavClient.

Thanks for the heads up! I'll dive into this :P

I haven't tried this but I believe it's possible to get upload progress by passing an HttpClient instance with a custom message handler to the WebDavClient.

Thanks for the heads up! I'll dive into this :P

Hi @unconnected, did you manage it? Looking for the same!


Feature/ASAA-19/PR Screen

np this is fun haha :)

@br-jared-stockton np.  This is fun haha :)


ndt7: some tests give significantly lower results

This is a pattern I've consistently noticed by running tests from my FTTH Internet connection (~300Mbit/s DL, ~100Mbit/s UL).
The vast majority of tests will run just fine, giving the expected results. Then, occasionally, a download test will begin at a much lower speed than usual, and the speed further lowers during the test. Counter-flow messages are much sparser than usual when this happens.

The fact that the test happening immediately before the "cursed" one and immediately afterwards show the expected results even using the same server makes me think this might be a bug in the protocol implementation. Feel free to close this issue if that's not the case. I am available for further testing if needed.

From the spec:

BytesSent and BytesRetrans can be used to compute the percentage of bytes overall that have been retransmitted.

From the last TCPInfo sample:
{"BytesSent": 23292545, "BytesRetrans": 5554240}

This gives us the figure that 24% of the bytes have been retransmitted.
Also from TCPInfo (same sample):
{"RTT": 23948, "RTTVar": 1336, "MinRTT": 2052}

This is surprising me. The MinRTT is unreasonable low (2 ms). You have a large RTT (24 ms). So there must be queue going on, which is consistent with what we see from other TCPInfo fields.
Can I see the TCPInfo of a good expriment?
I sincerely wonder whether in this experiment there is end-to-end connectivity.

 There is something in the middle that is confusing BBR to believe that the MinRTT is much lower than expected.

What's more, the first BBR sample of MinRTT looks reasonable. After a while, instead, we see that the MinRTT is dropping to a value that is clearly unreasonable. (Where reasonable and unreasonable are defined as the typical ping seen by @robertodauria towards destinations.)


Front-end packaging error

Search before asking

[X] I had searched in the issues and found no similar issues.

What happened
Front-end packaging error

What you expected to happen
Added code review mechanism?
How to reproduce
[root@LAPTOP-KAHM10NT dolphinscheduler-ui]# npm run build:prod

<EMAIL_ADDRESS>build:prod
vue-tsc --noEmit && vite build --mode production

src/App.tsx:74:33 - error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
74           {this.isRouterAlive ?  : ''}
~~~~~~~~~~~~~~~
src/components/chart/modules/Bar.tsx:90:7 - error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
90       <div
~~~~
91         ref='barChartRef'
~~~~~~~~~~~~~~~~~~~~~~~~~
...
95         }}
~~~~~~~~~~
96       />
~~~~~~~~
src/components/chart/modules/Gauge.tsx:132:7 - error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
Anything else
No response
Version
3.2.x
Are you willing to submit PR?

[ ] Yes I am willing to submit a PR!

Code of Conduct

[X] I agree to follow this project's Code of Conduct

Please correct the title and describe the issue clearly. I don't quite understand what your question is. @liusy-bd

请更正标题并清楚描述问题。我不太明白你的问题是什么。@liusy-bd

dolphinscheduler-ui    run 【npm run build:prod】 error

please have a try like this
pnpm run build:prod

It is OK on my env.

请尝试一下
pnpm run build:prod

在我的环境中这是正常的。

Same error

请尝试一下
pnpm run build:prod

在我的环境中这是正常的。

It's okay that I removed the type check,  That seems to be the problem


backgroundColor flips with reversed y-axis ticks on a line graph

Hello, I'm using backgroundColor fill with reversed ticks on the y-axis.  I'm trying to represent teams' rankings within a league, so the lower they rank 1-10 the better.  My line graph with a reversed y-axis shows teams that move from 8th to 5th to 3rd as moving up, which is my desired outcome.  However, when I add a backgroundColor fill, the area it fills is flipped as well.  The area above the line is now filled with my selected backgroundColor, but I want the area below my line down to the x-axis to be filled with a backgroundColor.   I've created a jsFiddle for reference here.  Is there any way to switch this?
Here is the bit of code I'm using to format my chart:
var data = {
		labels : ["10/17/17", "10/18/17", "10/19/17", "10/20/17", "10/21/17", "10/22/17", "10/23/17"],
		datasets : [
			{
			        label : "Average Rank",
				data : average.Jazz,
				borderColor : "#0C2340",
                                backgroundColor : gradientStroke,
                                fill : true,
				lineTension : 0,
				pointRadius : 0}
				]
			};

			var options = {
				legend : {
					display : true,
					position : "top"
				},
                maintainAspectRatio: true,
                scales: {
                                    xAxes: [{
                                        gridLines: {
                                            display: false
                                            },
                                    }],
                                    yAxes: [{
                                        ticks: {
                                            beginAtZero:false,
                                            reverse: true,
                                            min: 0,
                                            max: 35,
                                            stepSize: 5
                                        }
                                    }]
                                }
			};

Thanks in advance for any help provided!

this worked like a charm.  appreciate it Simon!


User don't know the green ones are button, better make it looks more like a button

Do we have any update on usability for this?


Fixes edit comment checkbox. Pharo issue #10985

Fixes edit class comment checkbox behavior. Pharo issue #10985
https://github.com/pharo-project/pharo/issues/10985

This PR should be merged.


cpp_redis can't correctly handle "Null reply".

Hi.
I use transactions with redis.
When using WATCH, EXEC can return a "Null reply" if the execution was aborted. But in this case callback is not triggered.
Steps.

Call WATCH for some "key".
From other client call SET "key" ...
From first app call MULTI and EXEC.
Last call of sync_commit() return, but callback for EXEC is not triggered.

Then I have tried to debug cpp_redis and found that it can't process reply like "*-1\r\n".
It throw exception in
bool array_builder::fetch_array_size(std::string& buffer)
Thanks.

Hi,
Thanks for the issue, I'll make the appropriate changes as soon as possible!
Best

Hi,
This was indeed an incorrect implementation of the protocol on my side.
Array with negative size were considered as invalid replies from the server.
I now pushed a fix which returns a null reply instead.
Please feel free to re-open the issue if necessary.
Best


convert functions and CLI

Closes #

What has been done to verify that this works as intended?
Local tests for functions and cli have been passed
Why is this the best possible solution? Were any other approaches considered?
for functions: use the tf2onnx api is good for framework version decoupling. when the stable version of tf2onnx SDK comes out,
the subprocess will be replaced.
for test: basically similar to existed tests, the model and inputs are download automatically.
for CLI : because the convert functions have different inputs args and output results, i set branches for every single converting action funcs.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
When there is a regression, the 3 functions and convert CLI will become invalid without any derived problems.
Does this change require updates to documentation?
yes, the cli wiki need a revise to add a intro to convert CLI: support tf2onnx, keras2onnx, tf2tfs, xgboost2onnx, xgboost2torch
Before submitting this PR, please make sure you have:

[x] run python -m pytest tests/ and confirmed all checks still pass.
[x] verified that any code or assets from external sources are properly credited.

Although  xgt.save_model() way is recommended in most scenarios because of version matching, I believe the pickle way is better in converting scenario. As the save_model way needs to know how the model is originally trained  which in the pipeline the user of converter probably have no idea of. And xgt.save_model is incompatible with the existing converting function. So though there is security concern about the use of pickle, I recommend to use it with a security warning notice to the user.

Also, note that test_onnx_conversion.py::TestONNXConverter::test_onnx_to_pytorch pytest has a bug, the fail Build / test (pull_request) is not caused by your code.

There is some issues with IOShape inputs, I am fixing it.

codacy warns about the use of eval(), while elsewhere in this project eval() is used by authors. So for the code style consistency I did not follow the instruction of codacy.

codacy warns about the use of eval(), while elsewhere in this project eval() is used by authors. So for the code style consistency I did not follow the instruction of codacy.

Use # nosec comment at the same line of the code to disable Codacy at that line.

codacy warns about the use of eval(), while elsewhere in this project eval() is used by authors. So for the code style consistency I did not follow the instruction of codacy.

Use # nosec comment at the same line of the code to disable Codacy at that line.

Can you delete this policy in Codacy @YuanmingLeee ? And also remove the nosec comments, it is related to the open source infrastructure and should not appear in the source.