Skip to content
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

feat(websockets): subscribe to closed client connections via ServerEvent.closeClient #252

Merged
merged 4 commits into from
Apr 28, 2020

Conversation

JozefFlakus
Copy link
Member

@JozefFlakus JozefFlakus commented Apr 24, 2020

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #160

What is the new behavior?

  • @marblejs/websockets - check and kill incoming connections either via HTTP server upgrade event or WebSocket connection event
  • @marblejs/websockets - subscribe to closed client connections via ServerEvent.closeClient ("close_client")
  • @marblejs/websockets - additional status logging middleware for detecting closed client connections
  • @marblejs/websockets - corrected predicates for noServer option
  • @marblejs/websockets - deprecated connection$attribute in WebSocketServerConfig
  • @marblejs/websockets - refactored integration tests (removed leaky "testbed")
const connection$: WsServerEffect = (event$) =>
  event$.pipe(
    matchEvent(ServerEvent.connection),
    map(event => event.payload),
    tap(({ client }) => {
      // check if connections should be closed
      client.close();
    }),
  );

const closeClient$: WsServerEffect = (event$) =>
  event$.pipe(
    matchEvent(ServerEvent.closeClient),
    map(event => event.payload),
    tap(({ client }) => {
      // react accordingly
    }),
  );

export const webSocketServer = createWebSocketServer({
  listener: webSocketListener({
    middlewares: [...],
    effects: [...],
  }),
  event$: combineEffects(
    connection$,
    closeClient$,
  ),
});

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@JozefFlakus JozefFlakus added enhancement New feature or request scope: websockets Relates to @marblejs/websockets package labels Apr 24, 2020
@JozefFlakus JozefFlakus added this to the 3.1 milestone Apr 24, 2020
@codecov
Copy link

codecov bot commented Apr 24, 2020

Codecov Report

Merging #252 into master will increase coverage by 0.29%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   95.07%   95.37%   +0.29%     
==========================================
  Files         147      147              
  Lines        2519     2527       +8     
  Branches      331      335       +4     
==========================================
+ Hits         2395     2410      +15     
+ Misses        120      113       -7     
  Partials        4        4              
Impacted Files Coverage Δ
.../src/transport/strategies/redis.strategy.helper.ts 100.00% <ø> (ø)
...ebsockets/src/server/websocket.server.interface.ts 100.00% <ø> (ø)
...es/websockets/src/server/websocket.server.event.ts 80.76% <75.00%> (+8.04%) ⬆️
packages/websockets/src/server/websocket.server.ts 96.42% <93.33%> (+0.97%) ⬆️
packages/core/src/http/error/http.error.model.ts 66.66% <100.00%> (ø)
...ssaging/src/transport/strategies/redis.strategy.ts 97.18% <100.00%> (-2.82%) ⬇️
...es/websockets/src/+internal/websocket.test.util.ts 100.00% <100.00%> (ø)
packages/websockets/src/index.ts 100.00% <100.00%> (ø)
.../middlewares/websockets.statusLogger.middleware.ts 100.00% <100.00%> (+17.39%) ⬆️
... and 4 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 b9a7832...9e4a7a1. Read the comment docs.

@JozefFlakus JozefFlakus changed the title feat(websockets): detect closed connections feat(websockets): subscribe to closed client connections via ServerEvent.closeClient Apr 25, 2020
@JozefFlakus JozefFlakus merged commit f3204b9 into master Apr 28, 2020
@JozefFlakus JozefFlakus deleted the feat/websocket-detect-closed-connections branch April 28, 2020 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request scope: websockets Relates to @marblejs/websockets package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant