-
Notifications
You must be signed in to change notification settings - Fork 4
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
Bug/#315 unsubscribe, subscribe에 queue 적용 #337
Changes from all commits
6c9b493
c394e8c
37ba795
06e6551
ab344c5
8cc4bb9
01feb88
3896e9f
bddda77
c97e4e0
7abd31f
9c8dfeb
bec4d72
c9537d2
0e3d0c5
acc1117
077b6a2
4016bd6
8c13646
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import { Inject, Injectable } from '@nestjs/common'; | |
import { | ||
ConnectedSocket, | ||
MessageBody, | ||
OnGatewayDisconnect, | ||
SubscribeMessage, | ||
WebSocketGateway, | ||
WebSocketServer, | ||
|
@@ -17,11 +18,13 @@ import { LiveData } from '@/scraper/openapi/liveData.service'; | |
pingTimeout: 5000, | ||
}) | ||
@Injectable() | ||
export class StockGateway { | ||
export class StockGateway implements OnGatewayDisconnect { | ||
@WebSocketServer() | ||
server: Server; | ||
private readonly mutex = new Mutex(); | ||
|
||
private readonly users: Map<string, string> = new Map(); | ||
|
||
constructor( | ||
private readonly liveData: LiveData, | ||
@Inject('winston') private readonly logger: Logger, | ||
|
@@ -32,40 +35,42 @@ export class StockGateway { | |
@MessageBody() stockId: string, | ||
@ConnectedSocket() client: Socket, | ||
) { | ||
client.join(stockId); | ||
|
||
await this.mutex.runExclusive(async () => { | ||
const connectedSockets = await this.server.to(stockId).fetchSockets(); | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try-catch로 묶으신 이유가 궁금합니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 현재 try-catch로 에러처리하는 부분이 따로 없고, 외부 유저와 connection하는 부분이다 보니 예기치 못한 에러가 발생할 수 있다고 생각했습니다 |
||
client.join(stockId); | ||
this.users.set(client.id, stockId); | ||
|
||
if (connectedSockets.length > 0 && !this.liveData.isSubscribe(stockId)) { | ||
await this.liveData.subscribe(stockId); | ||
this.logger.info(`${stockId} is subscribed`); | ||
} | ||
}); | ||
await this.mutex.runExclusive(async () => { | ||
const connectedSockets = await this.server.to(stockId).fetchSockets(); | ||
|
||
client.on('disconnecting', () => { | ||
client.rooms.delete(client.id); | ||
const stocks = Array.from(client.rooms.values()); | ||
for (const stock of stocks) { | ||
this.handleDisconnectStock(stock); | ||
} | ||
}); | ||
if ( | ||
connectedSockets.length > 0 && | ||
!this.liveData.isSubscribe(stockId) | ||
) { | ||
await this.liveData.subscribe(stockId); | ||
this.logger.info(`${stockId} is subscribed`); | ||
} | ||
}); | ||
|
||
client.emit('connectionSuccess', { | ||
message: `Successfully connected to stock room: ${stockId}`, | ||
stockId, | ||
}); | ||
client.emit('connectionSuccess', { | ||
message: `Successfully connected to stock room: ${stockId}`, | ||
stockId, | ||
}); | ||
} catch (e) { | ||
const error = e as Error; | ||
this.logger.warn(error.message); | ||
client.emit('error', error.message); | ||
client.disconnect(); | ||
} | ||
} | ||
|
||
async handleDisconnectStock(stockId: string) { | ||
await this.mutex.runExclusive(async () => { | ||
const connectedSockets = await this.server.in(stockId).fetchSockets(); | ||
|
||
if (connectedSockets.length === 0) { | ||
async handleDisconnect(client: Socket) { | ||
const stockId = this.users.get(client.id); | ||
if (stockId) { | ||
await this.mutex.runExclusive(async () => { | ||
await this.liveData.unsubscribe(stockId); | ||
this.logger.info(`${stockId} is unsubscribed`); | ||
} | ||
}); | ||
this.users.delete(client.id); | ||
}); | ||
} | ||
} | ||
|
||
onUpdateStock( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shift하면 비용이 n이라 비효율적이라 큐를 쓰는게 좋긴하지만, JS에서는 직접 구현해야되는게 아쉽네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 너무 아쉽습니다. 큐 말고 더 좋은 방법이 있을까 해서 onModuleInit도 고려했는 데, websocket을 열 수 있는 개수를 런타임때야 알 수 있으니 어쩔 수 없었다고 생각합니다.