proto-socket/agent-task/dart_close_fix/code_review_0.md

3 KiB

Code Review Reference — REVIEW_DART_API

개요

task=dart_close_fix, plan=0, tag=REVIEW_DART_API

구현 항목별 완료 여부

항목 완료 비고
[REVIEW_DART_API-1] sendRequest pending future hang 수정 [x] Communicator.cancelPendingRequests() 추가 및 BaseClient.close()에서 호출
[REVIEW_DART_API-2] onDisconnectedClient unawaited close 명시 [x] TCP/WS 서버 disconnect 콜백에서 unawaited(client.close()) 사용
[REVIEW_DART_API-3] disconnect 통보 순서 Go 기준 정렬 [x] transport close 완료 후 disconnect listener 통보

계획 대비 변경 사항

계획서 그대로 구현.

테스트에서는 @protected 멤버 접근 analyzer 경고를 피하기 위해 _FakeCommunicator.closeForTest() wrapper를 추가했다.

주요 설계 결정

  • pending request 취소는 _pendingRequests snapshot 생성 후 clear, 이후 각 completer에 StateError('connection closed')를 전달하는 순서로 구현했다.
  • BaseClient.close()isAlive = false, heartbeat 중지, pending request 취소, transport close, disconnect listener 통보 순서로 정렬했다.
  • onDisconnectedClientclose() 호출은 fire-and-forget 의도를 dart:asyncunawaited()로 명시했다.

리뷰어를 위한 체크포인트

  • dart/lib/src/communicator.dart:56cancelPendingRequests()가 pending request를 모두 StateError('connection closed')로 완료하는지 확인.
  • dart/lib/src/base_client.dart:31 — close 순서가 pending 취소 후 transport close, 이후 listener 통보인지 확인.
  • dart/lib/src/protobuf_server.dart:67 — TCP disconnect 콜백에서 unawaited(client.close()) 사용 확인.
  • dart/lib/src/ws_protobuf_server.dart:68 — WS disconnect 콜백에서 unawaited(client.close()) 사용 확인.
  • dart/test/communicator_test.dart:66 — close 후 pending sendRequest future가 StateError로 완료되는 회귀 테스트 확인.

검증 결과

$ cd dart && dart analyze
Analyzing dart...
No issues found!

$ cd dart && dart test test/communicator_test.dart
00:00 +0: loading test/communicator_test.dart
00:00 +0: Communicator protocol guards response typeName mismatch completes sendRequest with error
00:00 +1: Communicator protocol guards close 후 sendRequest는 StateError로 완료된다
00:00 +2: Communicator protocol guards cannot register addRequestListener for a type already using addListener
00:00 +3: Communicator protocol guards cannot register addListener for a type already using addRequestListener
00:00 +4: All tests passed!

$ cd dart && dart format lib/src/communicator.dart lib/src/base_client.dart lib/src/protobuf_server.dart lib/src/ws_protobuf_server.dart test/communicator_test.dart
Formatted lib/src/communicator.dart
Formatted 5 files (1 changed) in 0.11 seconds.

$ cd dart && dart test
00:15 +40: All tests passed!

$ cd dart && dart analyze
Analyzing dart...
No issues found!