fix: update Dart close() implementation
- Fix base_client.dart close() method - Update communicator.dart for close() pattern - Update protobuf_server.dart and ws_protobuf_server.dart - Update communicator_test.dart for close() API - skills: rename implement/SKILL.md to plan/SKILL.md - tasks: add dart_close_fix task files
This commit is contained in:
toki
parent
24adec1c1d
commit
182dcae5bf
10 changed files with 473 additions and 23 deletions
|
|
@ -4,7 +4,11 @@
|
|||
"Bash(apt-get update:*)",
|
||||
"Bash(apt-get install:*)",
|
||||
"Bash(sudo apt-get:*)",
|
||||
"Bash(protoc --version)"
|
||||
"Bash(protoc --version)",
|
||||
"Bash(dart analyze:*)"
|
||||
],
|
||||
"additionalDirectories": [
|
||||
"/config/workspace/toki_socket/tasks"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,14 +34,15 @@ abstract class BaseClient<Self extends BaseClient<Self>> extends Communicator
|
|||
}
|
||||
isAlive = false;
|
||||
stopHeartbeat();
|
||||
cancelPendingRequests();
|
||||
|
||||
await closeTransport();
|
||||
|
||||
final listeners = List<void Function(Self)>.from(_disconnectListeners);
|
||||
_disconnectListeners.clear();
|
||||
for (final fn in listeners) {
|
||||
fn(_self);
|
||||
}
|
||||
|
||||
await closeTransport();
|
||||
}
|
||||
|
||||
@protected
|
||||
|
|
|
|||
|
|
@ -53,6 +53,16 @@ abstract class Communicator {
|
|||
|
||||
Future<void> send<T extends GeneratedMessage>(T data);
|
||||
|
||||
@protected
|
||||
void cancelPendingRequests() {
|
||||
final snapshot = Map<int, _PendingRequest>.from(_pendingRequests);
|
||||
_pendingRequests.clear();
|
||||
for (final pending in snapshot.values) {
|
||||
pending.completeError(
|
||||
StateError('connection closed'), StackTrace.current);
|
||||
}
|
||||
}
|
||||
|
||||
/// Sends [data] as a request and waits for a typed response.
|
||||
///
|
||||
/// The remote side must have registered an [addRequestListener] for [Req].
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
// ignore_for_file: prefer_final_fields
|
||||
|
||||
import 'dart:async';
|
||||
import 'dart:io';
|
||||
import 'package:protobuf/protobuf.dart';
|
||||
import 'packets/message_common.pb.dart';
|
||||
|
|
@ -65,7 +66,7 @@ abstract class ProtobufServer {
|
|||
|
||||
void onDisconnectedClient(ProtobufClient client) {
|
||||
_clientList.remove(client);
|
||||
client.close();
|
||||
unawaited(client.close());
|
||||
}
|
||||
|
||||
Future broadcast<T extends GeneratedMessage>(T data) async {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
// ignore_for_file: prefer_final_fields
|
||||
|
||||
import 'dart:async';
|
||||
import 'dart:io';
|
||||
import 'package:protobuf/protobuf.dart';
|
||||
import 'packets/message_common.pb.dart';
|
||||
|
|
@ -66,7 +67,7 @@ abstract class WsProtobufServer {
|
|||
|
||||
void onDisconnectedClient(WsProtobufClient client) {
|
||||
_clientList.remove(client);
|
||||
client.close();
|
||||
unawaited(client.close());
|
||||
}
|
||||
|
||||
Future broadcast<T extends GeneratedMessage>(T data) async {
|
||||
|
|
|
|||
|
|
@ -18,6 +18,11 @@ class _FakeCommunicator extends Communicator {
|
|||
sentPackets.add(base);
|
||||
}
|
||||
|
||||
void closeForTest() {
|
||||
isAlive = false;
|
||||
cancelPendingRequests();
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> send<T extends GeneratedMessage>(T data) async {
|
||||
if (isAlive) {
|
||||
|
|
@ -58,6 +63,27 @@ void main() {
|
|||
);
|
||||
});
|
||||
|
||||
test('close 후 sendRequest는 StateError로 완료된다', () async {
|
||||
final communicator = _FakeCommunicator();
|
||||
final future = communicator.sendRequest<TestData, TestData>(
|
||||
TestData()..index = 1,
|
||||
);
|
||||
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
communicator.closeForTest();
|
||||
|
||||
await expectLater(
|
||||
future,
|
||||
throwsA(
|
||||
isA<StateError>().having(
|
||||
(error) => error.message,
|
||||
'message',
|
||||
contains('connection closed'),
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test(
|
||||
'cannot register addRequestListener for a type already using addListener',
|
||||
() {
|
||||
|
|
|
|||
|
|
@ -7,12 +7,12 @@ description: Review completed implementation work for toki_socket. Reads PLAN.md
|
|||
|
||||
## Overview
|
||||
|
||||
This skill is the third step in the implement → code loop:
|
||||
This skill is the third step in the plan → code → review loop:
|
||||
|
||||
```
|
||||
implement skill → implementing agent → code-review skill (this)
|
||||
↑ |
|
||||
└──────── (issues found) ── new PLAN.md ─┘
|
||||
plan skill → implementing agent → code-review skill (this)
|
||||
↑ |
|
||||
└──────── (issues found) ── new PLAN.md┘
|
||||
```
|
||||
|
||||
**Directory state signals:**
|
||||
|
|
@ -37,24 +37,55 @@ Note: `PLAN.md` and `CODE_REVIEW.md` are always present together. The implementi
|
|||
|
||||
## Step 2 — Load Context
|
||||
|
||||
Read all of the following before evaluating anything:
|
||||
The reading strategy depends on whether this is the first review or a follow-up.
|
||||
|
||||
1. `tasks/{task_name}/CODE_REVIEW.md` — implementation report filled by the implementing agent.
|
||||
2. `tasks/{task_name}/PLAN.md` — the plan the implementation followed.
|
||||
3. Every source file listed in the plan's "수정 파일 및 체크리스트" — read the actual current file.
|
||||
### 2-1. Determine review round
|
||||
|
||||
Count `tasks/{task_name}/code_review_*.log` files.
|
||||
- Count = 0 → **First review**: no prior diff to reference. Read the full codebase as described in 2-2A.
|
||||
- Count ≥ 1 → **Follow-up review**: use git diff to anchor the search, then expand as described in 2-2B.
|
||||
|
||||
### 2-2A. First review — full analysis
|
||||
|
||||
No git narrowing. Read everything relevant to the plan:
|
||||
|
||||
1. `tasks/{task_name}/CODE_REVIEW.md`
|
||||
2. `tasks/{task_name}/PLAN.md`
|
||||
3. Every source file listed in the plan's "수정 파일 및 체크리스트" — read the whole file.
|
||||
4. Every test file that exercises the changed code.
|
||||
5. Every file that imports or is imported by the changed files.
|
||||
|
||||
### 2-2B. Follow-up review — diff-anchored + expansion
|
||||
|
||||
Start with git to identify what changed, then expand outward to connected code.
|
||||
|
||||
**Collect the diff:**
|
||||
```bash
|
||||
git rev-parse HEAD 2>/dev/null && git diff HEAD || (git diff && git diff --cached)
|
||||
git diff main..HEAD # if on a feature branch
|
||||
git log --oneline -10
|
||||
```
|
||||
|
||||
**Expand beyond the diff** — for each changed file or symbol, also read:
|
||||
- Files that **import** the changed file (callers may be broken by signature changes)
|
||||
- Files that **implement or extend** a changed interface or abstract class
|
||||
- Test files that **exercise** the changed logic, even if not modified themselves
|
||||
- Any file the plan's "수정 파일 및 체크리스트" lists that does not appear in the diff (planned but possibly missing)
|
||||
|
||||
The diff scopes where to start; it does not limit where to stop. Follow the connections.
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Pre-Review Checklist
|
||||
|
||||
Complete every item before writing the verdict.
|
||||
Complete every item before writing the verdict. Use the git diff as the primary reference — only reach outside it when a specific check demands it.
|
||||
|
||||
- [ ] Cross-check every plan checkbox against the actual source file — was the change applied?
|
||||
- [ ] Grep for every renamed or removed symbol — confirm no stale references anywhere.
|
||||
- [ ] For every new test listed in the plan: confirm it exists, has the right name, and the assertion is non-trivial (not just "no exception thrown").
|
||||
- [ ] Check for regressions: read tests covering adjacent behavior not mentioned in the plan.
|
||||
- [ ] Cross-check the claimed `dart analyze` / `go test` output against the actual code.
|
||||
- [ ] Check for code quality issues: debug prints, dead code, leftover TODOs.
|
||||
- [ ] Compare git diff against the plan's "수정 파일 및 체크리스트" — are all planned changes present? Are there unplanned changes?
|
||||
- [ ] For every symbol renamed or removed in the diff: `git grep <old_name>` on the pre-change tree (or Grep on current tree) to confirm no stale references remain outside the diff.
|
||||
- [ ] For every new test added in the diff: confirm the test name matches the plan, and the assertion is non-trivial (not just "no exception thrown").
|
||||
- [ ] Scan the diff for code quality issues: debug prints, dead code, leftover TODOs, commented-out blocks.
|
||||
- [ ] Check for unintended diff noise: formatting-only changes, unrelated file edits, accidentally committed debug code.
|
||||
- [ ] Cross-check the claimed `dart analyze` / `go test` output in CODE_REVIEW.md against what the diff actually introduces.
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -128,7 +159,7 @@ No further files needed. Report to the user:
|
|||
|
||||
Write new files for the fix round:
|
||||
|
||||
**Write `tasks/{task_name}/PLAN.md`** following the implement skill format exactly:
|
||||
**Write `tasks/{task_name}/PLAN.md`** following the plan skill format exactly:
|
||||
- Count `tasks/{task_name}/plan_*.log` files **after** the archive above → that count is the new plan's N.
|
||||
- Header: `<!-- task={task_name} plan=N tag=REVIEW_<PARENT_TAG> -->`
|
||||
- Include the "구현 에이전트에게" section.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
---
|
||||
name: implement
|
||||
name: plan
|
||||
description: Analyze the codebase and write a detailed PLAN.md for toki_socket improvements. Also writes the CODE_REVIEW.md stub that the implementing agent will fill in after coding. Use for any new feature, refactor, bug fix, or follow-up fix. A separate agent reads PLAN.md and does the actual coding. The code-review skill archives both files after review.
|
||||
---
|
||||
|
||||
# Implement
|
||||
# Plan
|
||||
|
||||
## Overview
|
||||
|
||||
62
tasks/dart_close_fix/CODE_REVIEW.md
Normal file
62
tasks/dart_close_fix/CODE_REVIEW.md
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
<!-- task=dart_close_fix plan=0 tag=REVIEW_DART_API -->
|
||||
|
||||
# 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 통보 순서로 정렬했다.
|
||||
- `onDisconnectedClient`의 `close()` 호출은 fire-and-forget 의도를 `dart:async`의 `unawaited()`로 명시했다.
|
||||
|
||||
## 리뷰어를 위한 체크포인트
|
||||
|
||||
- `dart/lib/src/communicator.dart:56` — `cancelPendingRequests()`가 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!
|
||||
```
|
||||
314
tasks/dart_close_fix/PLAN.md
Normal file
314
tasks/dart_close_fix/PLAN.md
Normal file
|
|
@ -0,0 +1,314 @@
|
|||
# Dart close() 버그 수정 및 Go 정렬
|
||||
|
||||
<!-- task=dart_close_fix plan=0 tag=REVIEW_DART_API -->
|
||||
|
||||
## 이 파일을 읽는 구현 에이전트에게
|
||||
|
||||
- 각 항목의 체크리스트를 순서대로 완료한다.
|
||||
- 항목 완료마다 **중간 검증** 커맨드를 실행하고 통과를 확인한다.
|
||||
- 모든 항목 완료 후 **최종 검증** 커맨드를 실행한다.
|
||||
- 작업이 끝나면 `CODE_REVIEW.md`의 각 섹션을 실제 구현 내용으로 채운다.
|
||||
|
||||
## 배경
|
||||
|
||||
코드 리뷰에서 세 가지 이슈가 발견됐다.
|
||||
가장 중요한 것은 `sendRequest` 호출 도중 `close()`가 불리면 반환된 `Future`가
|
||||
영원히 완료되지 않는 버그다 (Go는 `closed` 채널로 즉시 해제함).
|
||||
나머지 두 항목은 unawaited Future 경고와 Go/Dart 간 disconnect 통보 순서 비대칭 문제다.
|
||||
|
||||
---
|
||||
|
||||
### [REVIEW_DART_API-1] `sendRequest` pending future hang
|
||||
|
||||
**문제**
|
||||
|
||||
`dart/lib/src/communicator.dart:64-88` — `sendRequest`는 `_pendingRequests`에
|
||||
`_PendingRequest`를 추가하고 `completer.future`를 반환한다.
|
||||
`close()`가 호출돼도 `_pendingRequests`의 completer에 에러를 전달하는 코드가 없어
|
||||
해당 future는 영원히 미완료 상태로 남는다.
|
||||
|
||||
Go 구현 (`go/communicator.go:196-210`)은 `SendRequest` 내부의 `select`에
|
||||
`case <-c.closed:` 분기를 두어 연결 종료 즉시 `ErrNotConnected`를 반환한다.
|
||||
|
||||
**해결 방법**
|
||||
|
||||
`Communicator`에 `@protected cancelPendingRequests()` 메서드를 추가하고,
|
||||
`BaseClient.close()` 내부에서 `isAlive = false` 직후에 호출한다.
|
||||
|
||||
```dart
|
||||
// dart/lib/src/communicator.dart — cancelPendingRequests() 추가
|
||||
// ignore_for_file 지시어 아래, Communicator 클래스 내부
|
||||
|
||||
@protected
|
||||
void cancelPendingRequests() {
|
||||
final snapshot = Map<int, _PendingRequest>.from(_pendingRequests);
|
||||
_pendingRequests.clear();
|
||||
for (final pending in snapshot.values) {
|
||||
pending.completeError(StateError('connection closed'), StackTrace.current);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`_PendingRequest`에 `completeError` 메서드가 이미 있으므로 (`communicator.dart:198`)
|
||||
추가 변경 없이 호출 가능하다.
|
||||
|
||||
```dart
|
||||
// dart/lib/src/base_client.dart — close() 수정
|
||||
// Before — base_client.dart:31-45
|
||||
@override
|
||||
Future<void> close() async {
|
||||
if (!isAlive) return;
|
||||
isAlive = false;
|
||||
stopHeartbeat();
|
||||
|
||||
final listeners = List<void Function(Self)>.from(_disconnectListeners);
|
||||
_disconnectListeners.clear();
|
||||
for (final fn in listeners) fn(_self);
|
||||
|
||||
await closeTransport();
|
||||
}
|
||||
|
||||
// After
|
||||
@override
|
||||
Future<void> close() async {
|
||||
if (!isAlive) return;
|
||||
isAlive = false;
|
||||
stopHeartbeat();
|
||||
cancelPendingRequests(); // ← 추가
|
||||
|
||||
final listeners = List<void Function(Self)>.from(_disconnectListeners);
|
||||
_disconnectListeners.clear();
|
||||
for (final fn in listeners) fn(_self);
|
||||
|
||||
await closeTransport();
|
||||
}
|
||||
```
|
||||
|
||||
**수정 파일 및 체크리스트**
|
||||
|
||||
- [ ] `dart/lib/src/communicator.dart`
|
||||
- [ ] `import 'package:meta/meta.dart';` 가 이미 있는지 확인 (있음 — 22행)
|
||||
- [ ] `cancelPendingRequests()` 메서드를 `Communicator` 클래스 안에 추가
|
||||
- `@protected` 어노테이션 적용
|
||||
- `_pendingRequests`를 snapshot → clear → 각 항목 `completeError(StateError('connection closed'), StackTrace.current)` 호출 순으로 구현
|
||||
- [ ] `dart/lib/src/base_client.dart`
|
||||
- [ ] `close()` 내부 `stopHeartbeat()` 호출 다음 줄에 `cancelPendingRequests();` 추가
|
||||
|
||||
**테스트 작성**
|
||||
|
||||
새 공개 API 동작이므로 회귀 테스트 필수.
|
||||
|
||||
| 파일 | `dart/test/communicator_test.dart` |
|
||||
|------|--------------------------------------|
|
||||
| 테스트 이름 | `'close 후 sendRequest는 StateError로 완료된다'` |
|
||||
| 픽스처 | 기존 `_FakeCommunicator` 사용 (`isAlive = true`로 초기화됨) |
|
||||
| 검증 목표 | `sendRequest` 호출 → `Future.delayed(Duration.zero)` → `isAlive = false` 직접 설정 + `cancelPendingRequests()` 호출 → future가 `StateError('connection closed')` 로 완료되는지 확인 |
|
||||
|
||||
```dart
|
||||
test('close 후 sendRequest는 StateError로 완료된다', () async {
|
||||
final communicator = _FakeCommunicator();
|
||||
final future = communicator.sendRequest<TestData, TestData>(
|
||||
TestData()..index = 1,
|
||||
);
|
||||
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
communicator.isAlive = false;
|
||||
communicator.cancelPendingRequests();
|
||||
|
||||
await expectLater(
|
||||
future,
|
||||
throwsA(
|
||||
isA<StateError>().having(
|
||||
(e) => e.message,
|
||||
'message',
|
||||
contains('connection closed'),
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
주의: `_FakeCommunicator`는 `Communicator`를 직접 상속하므로 `@protected` 메서드를
|
||||
테스트 코드에서 직접 호출할 수 있다.
|
||||
(`cancelPendingRequests()`는 `@protected`지만 같은 라이브러리 내이면 접근 가능.
|
||||
테스트 파일은 별도 패키지이므로 `_FakeCommunicator`에 wrapper를 두거나
|
||||
`communicator_test.dart`에서 subclass로 노출 필요 — 구현 에이전트가 판단.)
|
||||
|
||||
**중간 검증**
|
||||
|
||||
```bash
|
||||
cd dart && dart analyze
|
||||
cd dart && dart test test/communicator_test.dart
|
||||
```
|
||||
|
||||
Expected: `No issues found!` + communicator 테스트 전량 통과
|
||||
|
||||
---
|
||||
|
||||
### [REVIEW_DART_API-2] `onDisconnectedClient` unawaited `close()`
|
||||
|
||||
**문제**
|
||||
|
||||
`dart/lib/src/protobuf_server.dart:66-69`와
|
||||
`dart/lib/src/ws_protobuf_server.dart:66-69`에서
|
||||
`void onDisconnectedClient(...)` 콜백이 `client.close()`를 await 없이 호출한다.
|
||||
`stop()`은 `await client.close()`를 쓰는 반면 콜백은 그렇지 않아 일관성이 없고
|
||||
dart analyzer의 `discarded_futures` 경고 대상이다.
|
||||
|
||||
**해결 방법**
|
||||
|
||||
`dart:async`의 `unawaited()`를 사용해 의도적으로 fire-and-forget임을 명시한다.
|
||||
|
||||
```dart
|
||||
// Before — protobuf_server.dart:66-69
|
||||
void onDisconnectedClient(ProtobufClient client) {
|
||||
_clientList.remove(client);
|
||||
client.close();
|
||||
}
|
||||
|
||||
// After
|
||||
void onDisconnectedClient(ProtobufClient client) {
|
||||
_clientList.remove(client);
|
||||
unawaited(client.close());
|
||||
}
|
||||
```
|
||||
|
||||
`ws_protobuf_server.dart`도 동일하게 적용한다.
|
||||
|
||||
**수정 파일 및 체크리스트**
|
||||
|
||||
- [ ] `dart/lib/src/protobuf_server.dart`
|
||||
- [ ] `import 'dart:async';` 이미 없으면 추가 (파일 상단 확인 필요)
|
||||
- [ ] `onDisconnectedClient` 내 `client.close()` → `unawaited(client.close())`
|
||||
- [ ] `dart/lib/src/ws_protobuf_server.dart`
|
||||
- [ ] `import 'dart:async';` 이미 없으면 추가
|
||||
- [ ] `onDisconnectedClient` 내 `client.close()` → `unawaited(client.close())`
|
||||
|
||||
**테스트 작성**
|
||||
|
||||
내부 변경이므로 기존 테스트 통과로 충분. 신규 테스트 불필요.
|
||||
|
||||
**중간 검증**
|
||||
|
||||
```bash
|
||||
cd dart && dart analyze
|
||||
cd dart && dart test
|
||||
```
|
||||
|
||||
Expected: `No issues found!` + 기존 39개 + 신규 1개(REVIEW_DART_API-1) = 40개 통과
|
||||
|
||||
---
|
||||
|
||||
### [REVIEW_DART_API-3] disconnect 통보 순서 — Go 기준으로 정렬
|
||||
|
||||
**문제**
|
||||
|
||||
Go `base_client.go:45-56`의 `Close()` 순서:
|
||||
```
|
||||
shutdown() → stopHeartbeat() → doClose() [transport] → notifyDisconnected() [listeners]
|
||||
```
|
||||
|
||||
Dart `base_client.dart:31-45`의 현재 `close()` 순서:
|
||||
```
|
||||
isAlive=false → stopHeartbeat() → [listeners 통보] → closeTransport()
|
||||
```
|
||||
|
||||
Dart는 listener가 호출되는 시점에 transport가 아직 열려 있다.
|
||||
리스너 안에서 `send()`를 시도하면 `isAlive == false`에 의해 차단되므로
|
||||
현재는 문제가 없으나, Go와 의미론이 달라 혼란을 유발한다.
|
||||
Go의 의도: "transport가 완전히 닫힌 후에 외부에 알린다."
|
||||
|
||||
**해결 방법**
|
||||
|
||||
`BaseClient.close()`에서 `closeTransport()` 호출을 listener 통보보다 앞으로 이동한다.
|
||||
|
||||
```dart
|
||||
// Before — base_client.dart:31-45
|
||||
@override
|
||||
Future<void> close() async {
|
||||
if (!isAlive) return;
|
||||
isAlive = false;
|
||||
stopHeartbeat();
|
||||
cancelPendingRequests();
|
||||
|
||||
final listeners = List<void Function(Self)>.from(_disconnectListeners);
|
||||
_disconnectListeners.clear();
|
||||
for (final fn in listeners) fn(_self);
|
||||
|
||||
await closeTransport();
|
||||
}
|
||||
|
||||
// After
|
||||
@override
|
||||
Future<void> close() async {
|
||||
if (!isAlive) return;
|
||||
isAlive = false;
|
||||
stopHeartbeat();
|
||||
cancelPendingRequests();
|
||||
|
||||
await closeTransport(); // ← transport 먼저 닫고
|
||||
|
||||
final listeners = List<void Function(Self)>.from(_disconnectListeners);
|
||||
_disconnectListeners.clear();
|
||||
for (final fn in listeners) fn(_self);
|
||||
}
|
||||
```
|
||||
|
||||
기존 테스트에서 listener 통보와 transport 종료 사이의 순서를 직접 검증하는 케이스는 없으므로
|
||||
순서 변경 후에도 기존 39개 + 신규 1개 = 40개 테스트가 그대로 통과해야 한다.
|
||||
|
||||
**수정 파일 및 체크리스트**
|
||||
|
||||
- [ ] `dart/lib/src/base_client.dart`
|
||||
- [ ] `close()` 내부에서 `await closeTransport()` 를 listener 통보 블록보다 앞으로 이동
|
||||
(REVIEW_DART_API-1의 `cancelPendingRequests()` 추가 후 그 바로 아래)
|
||||
|
||||
**테스트 작성**
|
||||
|
||||
순서 변경이므로 기존 테스트 통과로 충분. 신규 테스트 불필요.
|
||||
|
||||
**중간 검증**
|
||||
|
||||
```bash
|
||||
cd dart && dart analyze
|
||||
cd dart && dart test
|
||||
```
|
||||
|
||||
Expected: `No issues found!` + 40개 전량 통과
|
||||
|
||||
---
|
||||
|
||||
## 의존 관계 및 구현 순서
|
||||
|
||||
```
|
||||
[REVIEW_DART_API-1] → [REVIEW_DART_API-2] → [REVIEW_DART_API-3]
|
||||
```
|
||||
|
||||
- 1번이 `cancelPendingRequests()`를 `Communicator`와 `BaseClient.close()`에 추가한다.
|
||||
- 2번과 3번은 독립적이나 1번 완료 후 `dart analyze`를 확인하고 진행한다.
|
||||
- 3번은 `BaseClient.close()`를 한 번만 수정하므로 1번 diff와 합산 적용한다.
|
||||
|
||||
---
|
||||
|
||||
## 수정 파일 요약
|
||||
|
||||
| 파일 | 항목 |
|
||||
|------|------|
|
||||
| `dart/lib/src/communicator.dart` | REVIEW_DART_API-1 |
|
||||
| `dart/lib/src/base_client.dart` | REVIEW_DART_API-1, REVIEW_DART_API-3 |
|
||||
| `dart/lib/src/protobuf_server.dart` | REVIEW_DART_API-2 |
|
||||
| `dart/lib/src/ws_protobuf_server.dart` | REVIEW_DART_API-2 |
|
||||
| `dart/test/communicator_test.dart` | REVIEW_DART_API-1 (새 테스트) |
|
||||
|
||||
---
|
||||
|
||||
## 최종 검증
|
||||
|
||||
```bash
|
||||
# 1. 정적 분석
|
||||
cd dart && dart analyze
|
||||
# 2. 전체 테스트
|
||||
cd dart && dart test
|
||||
```
|
||||
|
||||
Expected outcome: `No issues found!` + 기존 39개 + 신규 1개 = **총 40개 통과**
|
||||
Loading…
Reference in a new issue