CLEANUP: Replace assert statements with proper exception handling by f1v3-dev · Pull Request #1077 · naver/arcus-java-client

f1v3-dev · 2026-04-06T08:15:09Z

🔗 Related Issue

https://github.com/jam2in/arcus-works/issues/576

⌨️ What I did

assert 문을 중요도에 따라 분류하고, 중요도 높음 항목을 우선 처리합니다.

처리 기준

I/O Thread 내 서버 응답 검증 → IllegalStateException 으로 대체
null, 범위 검증 assert 제거 → RuntimeException(NPE, IndexOutOfBoundsException 등) 발생하므로 불필요
상위 조건으로 이미 보장된 assert 제거

jhpark816

리뷰 완료

oliviarla

리뷰 완료입니다.

jhpark816

일부 리뷰

f1v3-dev · 2026-04-21T03:12:00Z

transcoders 패키지

서버에서 온 외부 데이터 검증이며 I/O Thread에서 실행되지 않으므로 throw new IllegalStateException으로 교체했습니다.

파일	변경 내용
`TranscoderUtils`	`decodeInt/Long/Byte/Boolean`의 assert → `IllegalStateException`
`WhalinTranscoder`	`decodeBoolean`의 assert → `IllegalStateException`
`WhalinV1Transcoder`	`decodeByte/Integer/Float/Double/Boolean`의 assert → `IllegalStateException`

protocol/ascii 패키지

I/O Thread에서 실행되므로 케이스별로 판단했습니다.

assert 내용	판단	처리
`tmp != lookingFor` (`\r\n` 검증)	스트림 오염 → reconnect	`IllegalStateException`으로 교체
`"VALUE".equals(stuff[0])`	`startsWith("VALUE ")` 진입 조건으로 이미 보장	제거
`stuff.length == N`	위반 시 자연스럽게 `ArrayIndexOutOfBoundsException` 발생	제거
`key/data != null`, `readOffset <= data.length`	내부 불변식	제거
`default: assert false` (`\r\n` switch)	`lookingFor`는 `'\r'`, `'\n'`만 가능 — 도달 불가	제거
`initialize()` switch default 없음	하위 클래스가 `get/gets/gat/gats/mget/mgets`만 전달 — 도달 불가	제거

@oliviarla

위와 같이 변경하였습니다. 리뷰 부탁드립니다.

oliviarla

리뷰 완료입니다.

jhpark816

일부 리뷰

jhpark816 · 2026-04-21T07:59:10Z

+        commandBuilder.append(RN_STRING);
+        commandBuilder.append(keysString);
+        commandBuilder.append(RN_STRING);
+        break;


default 문에서 exception throw가 맞지 않나 생각합니다.

아래에 있는 코드를 수행한다면, 오히려 맞지 않기 때문입니다.

inittialize() 를 호출하는 하위 클래스에서 현재 switch-case 이외의 cmd 값을 넘기지 않습니다.

따라서 도달할 수 없는 코드라고 판단하여 default를 제거한 상태입니다.

default 문으로 기존대로 assert 추가해 두는 것이 좋겠습니다.

assert는 현재 코드 기준으로 false여야 합니다.
향후 코드 수정하더라도 false를 만족시켜야 함을 알 수 있으며,
위의 예시에서 그 외의 cmd가 없음을 명시하는 효과가 있습니다.

default: assert false : "Unknown command: " + cmd;

위와 같이 반영하도록 하겠습니다.

jhpark816

일부 리뷰

jhpark816 · 2026-04-21T10:31:20Z

+        commandBuilder.append(RN_STRING);
+        commandBuilder.append(keysString);
+        commandBuilder.append(RN_STRING);
+        break;


default 문으로 기존대로 assert 추가해 두는 것이 좋겠습니다.

assert는 현재 코드 기준으로 false여야 합니다.
향후 코드 수정하더라도 false를 만족시켜야 함을 알 수 있으며,
위의 예시에서 그 외의 cmd가 없음을 명시하는 효과가 있습니다.

jhpark816 · 2026-04-23T04:57:25Z

@oliviarla
수정된 사항이 있으니, 최종 리뷰 바랍니다.

f1v3-dev requested a review from oliviarla April 6, 2026 08:15

f1v3-dev self-assigned this Apr 6, 2026

jhpark816 requested a review from uhm0311 April 6, 2026 08:17

f1v3-dev force-pushed the cleanup/assert branch from 4f65194 to b9c310b Compare April 6, 2026 09:00

uhm0311 previously approved these changes Apr 6, 2026

View reviewed changes

jhpark816 reviewed Apr 7, 2026

View reviewed changes

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BaseGetOpImpl.java Outdated

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BaseGetOpImpl.java

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BaseGetOpImpl.java

f1v3-dev dismissed uhm0311’s stale review via 26325a1 April 8, 2026 02:51

f1v3-dev force-pushed the cleanup/assert branch from b9c310b to 26325a1 Compare April 8, 2026 02:51

oliviarla reviewed Apr 8, 2026

View reviewed changes

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BTreeFindPositionWithGetOperationImpl.java

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BTreeSortMergeGetOperationImpl.java Outdated

Comment thread src/main/java/net/spy/memcached/AddrUtil.java Outdated

f1v3-dev force-pushed the cleanup/assert branch from 26325a1 to 0474c46 Compare April 9, 2026 07:55

f1v3-dev requested a review from oliviarla April 9, 2026 07:57

f1v3-dev force-pushed the cleanup/assert branch from 0474c46 to 1357feb Compare April 9, 2026 08:01

oliviarla previously approved these changes Apr 9, 2026

View reviewed changes

jhpark816 reviewed Apr 9, 2026

View reviewed changes

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BaseGetOpImpl.java

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BTreeFindPositionWithGetOperationImpl.java

f1v3-dev dismissed oliviarla’s stale review via 4904d46 April 13, 2026 03:19

f1v3-dev force-pushed the cleanup/assert branch from 1357feb to 4904d46 Compare April 13, 2026 03:19

f1v3-dev requested a review from oliviarla April 13, 2026 08:47

f1v3-dev force-pushed the cleanup/assert branch from 4904d46 to cbfd531 Compare April 21, 2026 02:32

oliviarla reviewed Apr 21, 2026

View reviewed changes

Comment thread src/main/java/net/spy/memcached/protocol/ascii/BaseGetOpImpl.java

f1v3-dev force-pushed the cleanup/assert branch from cbfd531 to d839ec7 Compare April 21, 2026 06:48

oliviarla previously approved these changes Apr 21, 2026

View reviewed changes

oliviarla requested a review from jhpark816 April 21, 2026 06:52

jhpark816 reviewed Apr 21, 2026

View reviewed changes

jhpark816 reviewed Apr 22, 2026

View reviewed changes

f1v3-dev dismissed oliviarla’s stale review via 2bfae52 April 23, 2026 01:14

f1v3-dev force-pushed the cleanup/assert branch from d839ec7 to 2bfae52 Compare April 23, 2026 01:14

CLEANUP: Replace assert statements with proper exception handling

e12e318

f1v3-dev force-pushed the cleanup/assert branch from 2bfae52 to e12e318 Compare April 23, 2026 04:23

jhpark816 approved these changes Apr 23, 2026

View reviewed changes

jhpark816 requested a review from oliviarla April 23, 2026 04:50

oliviarla approved these changes Apr 23, 2026

View reviewed changes

jhpark816 merged commit f444e43 into naver:develop Apr 23, 2026
2 checks passed

Conversation

f1v3-dev commented Apr 6, 2026

🔗 Related Issue

⌨️ What I did

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oliviarla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

f1v3-dev commented Apr 21, 2026

transcoders 패키지

protocol/ascii 패키지

Uh oh!

oliviarla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

jhpark816 Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

f1v3-dev Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jhpark816 Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

f1v3-dev Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jhpark816 Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jhpark816 commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants