Skip to content

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Feb 21, 2022

Let's make ping pong less sync.

NONE

@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #8348 (8d338ec) into main (8fa98f9) will decrease coverage by 17.10%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8348       +/-   ##
===========================================
- Coverage   28.28%   11.17%   -17.11%     
===========================================
  Files          41       18       -23     
  Lines        3490      993     -2497     
===========================================
- Hits          987      111      -876     
+ Misses       2447      880     -1567     
+ Partials       56        2       -54     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-installer-raw-app ?
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
components-ws-proxy-app ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../installer/pkg/components/ws-manager/deployment.go
components/ws-proxy/pkg/proxy/workspacerouter.go
components/ws-proxy/pkg/proxy/routes.go
...s/installer/pkg/components/ws-manager/tlssecret.go
components/local-app/pkg/auth/pkce.go
components/installer/pkg/common/display.go
components/installer/pkg/common/objects.go
components/ws-proxy/pkg/proxy/config.go
components/ws-proxy/pkg/proxy/proxy.go
...components/ws-manager/unpriviledged-rolebinding.go
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fa98f9...8d338ec. Read the comment docs.

setPingSent(ws, Date.now());
ws.ping(); // if this fails it triggers a ws error, and fails the ws anyway

// note: decoupling by using `setImmediate` in order to offload to the following event loop iteration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -93,7 +101,14 @@ export class WsConnectionHandler implements Disposable {
});
ws.on('ping', (data: any) => {
// answer browser-side ping to conform RFC6455 (https://tools.ietf.org/html/rfc6455#section-5.5.2)
ws.pong(data);
// note: decoupling by using `setImmediate` in order to offload to the following event loop iteration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM, tested and works - thx! 🙏

@roboquat roboquat merged commit 268e67c into main Feb 21, 2022
@roboquat roboquat deleted the at/ping-pong branch February 21, 2022 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants