FIX: Send ERR_INTERNAL status when pipe error occurred

[oliviarla]

🔗 Related Issue

⌨️ What I did

• handleError에서 에러 발생 시와 동일하게 OperationStatus를 생성해 Future로 전달하도록 합니다.
• pipe error가 발생하지 않았을 경우에만 END / FAILED_END를 Future로 전달하도록 합니다.

[Copilot]

Pull Request Overview

This PR ensures that when a pipe error occurs, an ERR_INTERNAL status is sent and suppresses the usual END/FAILED_END statuses thereafter, aligning error handling with other failure paths.

• In PipeOperationImpl, it now sends an ERR_INTERNAL status on pipe errors and only emits END/FAILED_END if no exception was recorded.
• The test in BaseOpTest is updated to expect StatusCode.ERR_INTERNAL and to fail if any indexed status is received.
• Necessary imports were added to support the new status code and test assertions.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java	Imported StatusCode, added error-status emission in handleError, and gated END/FAILED_END on exception.
src/test/java/net/spy/memcached/protocol/ascii/BaseOpTest.java	Added StatusCode import, updated callbacks to assert ERR_INTERNAL, added fail() stub, and adjusted imports.

Comments suppressed due to low confidence (1)

src/test/java/net/spy/memcached/protocol/ascii/BaseOpTest.java:123

• Missing static import for assertEquals; add import static org.junit.jupiter.api.Assertions.assertEquals; at the top of the test file to allow this assertion to compile.

assertEquals(StatusCode.ERR_INTERNAL, status.getStatusCode());

[jhpark816]

@uhm0311 리뷰 바랍니다.

[jhpark816]

리뷰 완료

[jhpark816]

cb.receivedStatus()는 해당 연산을 완료할 때 1회 호출하는 것이 좋습니다.

BaseOperationImpl 클래스에 complete() 메소드 추가하는 PR 참고하세요.

• https://github.com/naver/arcus-java-client/pull/921/files#diff-94543a2dfabc4d5e887c9363369f411be96eec97de5feea9497746fd4e82121e

오히려, handleLine(line);을 추가적으로 호출함으로써 아래가 수행되게 하는 것이 맞을 것 같습니다.

• successAll = false; 설정
• cb.gotStatus(index, status) 호출하여 StatusCode.ERR_INTERNAL을 개별 failedResult로 등록

그리고, receivedStatus는 기존대로 아래 수행하는 것이 맞을 것 같습니다.

cb.receivedStatus((successAll) ? END : FAILED_END);

[oliviarla]

RESPONSE 오기 전 에러가 발생했을 때와 RESPONSE 온 후 에러가 발생해 PIPE_ERROR 상태가 되었을 때 두 경우 모두 동일하게 동작시키기 위함입니다. (전자의 경우 ERR_INTERNAL을 future의 operationStatus 필드에 담아두게 되어 있습니다.)

get() 호출 시에 두 경우 모두 예외가 던져지기 때문에 failedResult에는 exception 정보를 넣을 필요 없습니다.

complete() 추가를 고려한다면 차라리 handleLine에서 exception이 null인지 검사하는 코드를 제거하면 될 것 같습니다. 어차피 성공하지 못한 operationStatus가 이미 존재한다면 값이 덮어쓰여지지 않기 때문입니다.

[jhpark816]

handleLine에서 아래 코드처럼 처리하는 것이 좋겠습니다.

      OperationStatus status = (successAll) ? END : FAILED_END;
      cb.receivedStatus(exception != null ? ERR_INTERNAL : status);
      transitionState(OperationState.COMPLETE);

[oliviarla]

이유가 궁금합니다. 위와 같이 작성하게 되면 not piped 처리하는 부분과 END/PIPE_ERROR 처리하는 부분에 대해 일관성이 없어지게 되어 코드가 더 복잡해진다고 생각합니다.

[jhpark816]

cb.receivedStatus(status)는 그 의미 상 해당 op 수행을 완료할 시에 호출하는 메소드입니다.

현재 PR을 최근 코드 기준으로 rebase하고 나면,
해당 op 수행을 완료할 시에 아래 메소드를 호출하며, 이 메소드에서만 cb.receivedStatus(status) 호출합니다.

• op.complete(status) : 수행 완료
• op.cancel() : 수행 취소

op 수행 도중에 최종 status가 결정되더라도 그 즉시 cb.receivedStatus(status) 호출하는 것보다
그 status 보관하고 있다가 op.complete(status)에 넘겨서 완료하는 것이 일관적인 동작입니다.
(참고로, 현재 코드에 op 완료 전에 cb.receivedStatus(status) 호출하는 경우가 없습니다)

현재 PR을 최근 코드 기준으로 rebase하면, 아래 코드가 될 것입니다.

      OperationStatus status = (successAll) ? END : FAILED_END;
      complete(exception != null ? ERR_INTERNAL : status);

[jhpark816]

@uhm0311 리뷰하세요.