반쪽만 꺼진 가드

새 API 컨트롤러를 리뷰하다가 is_modify_lock 검사가 통째로 //로 주석 처리돼 있는 것을 봤다. 지우지 않고 주석으로 남긴 형태라 의도적으로 끈 것으로 보였다.

Table of contents

증상 — 상위는 꺼지고 하위는 켜져 있었다

is_modify_lock을 쓰는 곳을 grep -rn으로 전부 찾으니 서비스 계층과 작업 핸들러와 관리자 컨트롤러와 이전 버전 API 컨트롤러 네 곳이었다. 그중 작업 핸들러를 열어 보니 같은 검사가 살아 있어서 잠금이 걸린 상품이면 작업 생성을 막고 있었다.

if ($product->is_modify_lock) {
    return false;   // 잡 생성 차단
}

그러면 흐름이 이렇게 되는데 API 요청이 컨트롤러의 꺼진 검사를 통과해서 수정이 DB에 반영되고, 그다음 전송 작업을 만드는 자리에서 켜져 있는 검사에 걸려 차단된다. 우리 DB는 바뀌었는데 바깥으로는 안 나간 상태가 된다.

원인 — 조용한 불일치

이 조합이 나쁜 것은 어느 쪽도 오류로 나타나지 않기 때문이다. API는 성공을 돌려주고 우리 DB에는 값이 반영돼 있으니 호출한 쪽에서는 정상 처리된 것으로 읽힌다. 바깥 값이 안 바뀌었다는 것은 한참 뒤에 값을 대조하는 자리에서야 드러난다.

검사가 양쪽 다 켜져 있었으면 요청 단계에서 거부됐을 것이고, 양쪽 다 꺼져 있었으면 끝까지 반영됐을 것이다. 반쪽만 꺼진 상태가 가장 조용하게 어긋난다.

판단 기준 — 의도와 확인의 구분

이 주석 처리가 실수인지 판단인지를 보려고 이전 버전 컨트롤러를 열었더니 같은 자리가 같은 방식으로 주석 처리돼 있었다. 두 곳이 같은 모양이면 손이 미끄러진 것으로 보기는 어렵다.

기록에는 두 가지 방법이 있었는데 의도적 판단으로 단정해 적는 것과 확인한 것과 추정한 것을 나눠 적는 것이다. 뒤쪽을 골랐다. 이유는 코드가 같다는 것과 의도가 같다는 것은 다른 얘기이고 한쪽을 복사해서 만들었을 수도 있어서다. 대신 리뷰 의견이 단정 없이 길어지는 것을 감수했다. 상위가 주석 처리돼 있고 하위가 활성이며 그 결과 불일치가 생길 수 있다는 것까지는 코드로 확인한 사실이고, 그것이 의도적 판단이라는 것은 두 버전이 같다는 데서 나온 추정이다.

의도가 맞다고 해도 지금 상태는 그 의도대로 동작하지 않는다. API에서 잠금을 무시하고 수정을 허용하려던 것이라면 하위도 같이 꺼야 수정이 실제로 반영된다.

주의 — 나머지 둘의 성격

같은 리뷰에서 두 가지가 더 나왔다. 하나는 try/catch 블록이 통째로 주석 처리된 것이다. 예외가 나면 프레임워크 기본 오류 응답이 나가면서 스택 트레이스와 파일 경로와 쿼리가 호출한 쪽에 그대로 전달된다. 디버깅하려고 잠시 풀어 둔 것을 되돌리지 않은 형태로 보인다.

다른 하나는 ?: 삼항 조건이 뒤집힌 것이다. channel_product_id가 있으면 create이고 없으면 update로 돼 있는데 기준 구현은 반대였다. 논리로 따져도 식별자가 있다는 것은 이미 등록됐다는 뜻이니 갱신이 맞고 새 코드 쪽이 뒤집혀 있었다.

셋을 나란히 놓으면 원인이 다른데 첫째는 판단은 있었으나 적용 범위가 부족한 것이고, 둘째는 임시 조치를 안 되돌린 실수이며, 셋째는 논리 오류다.

교훈 — 주석 처리와 사용처 전수

앞의 둘에는 공통점이 있는데 둘 다 삭제가 아니라 // 주석 처리로 남아 있었다. 코드를 지우면 git diff에 삭제로 나타나 리뷰에서 눈에 띄는데, 주석으로 남기면 그 자리에 코드가 있으므로 훑어볼 때 살아 있는 것처럼 보인다. 주석 처리된 코드는 있는 것도 없는 것도 아닌 상태로 남는다.

첫 번째를 잡아 준 것은 사용처 전수 조사였다. 컨트롤러만 보면 검사가 꺼져 있다는 데서 판단이 끝나는데, is_modify_lock을 쓰는 자리를 전부 열어 봤기 때문에 하위가 켜져 있다는 것이 나왔다. 어떤 검사를 끄기로 했으면 그 값을 읽는 다른 자리도 같이 봐야 한다는 것이 이 건에서 남은 것이다.

정리

• 같은 is_modify_lock이 상위에서는 꺼지고 하위에서는 켜져 있을 수 있다
• 그러면 우리 DB는 바뀌고 바깥으로는 안 나가는 불일치가 생긴다
• 응답이 성공이라 호출한 쪽에서는 정상 처리로 읽힌다
• 반쪽만 꺼진 상태가 양쪽 다 꺼진 것보다 조용하게 어긋난다
• 두 곳이 같은 모양이면 의도로 추정되지만 확인한 사실과는 나눠 적는다
• 끄기로 했으면 그 값을 읽는 자리를 전부 꺼야 의도대로 동작한다
• 주석 처리된 코드는 훑어볼 때 살아 있는 것처럼 보인다
• try/catch를 풀어 두면 스택 트레이스와 경로가 그대로 나간다
• 검사를 끌 때는 그 값을 읽는 사용처를 전수로 본다