Skip to content

feat: make getCargo return T and throw when binding middleware is missing#227

Merged
yuchem2 merged 2 commits into
developfrom
CARGO-419
May 31, 2026
Merged

feat: make getCargo return T and throw when binding middleware is missing#227
yuchem2 merged 2 commits into
developfrom
CARGO-419

Conversation

@yuchem2

@yuchem2 yuchem2 commented May 29, 2026

Copy link
Copy Markdown
Member

논의 결과에 따라 getCargo가 undefined를 리턴하지 않도록 수정하고, 만약 bindingCargo가 실행되지 않은 상태로 실행되게 된다면 오류를 반환하도록 수정하였습니다.

@yuchem2 yuchem2 self-assigned this May 29, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

이번 풀리퀘스트는 getCargo 함수가 요청에서 cargo 객체를 찾지 못할 경우 에러를 던지도록 수정하고, 반환 타입을 T | undefined에서 T로 변경하며 관련 테스트 코드를 추가했습니다. 리뷰어는 req._cargonull일 가능성도 대비하여 cargo === undefined 대신 cargo == null을 사용해 더 안전하게 예외 처리를 하도록 개선안을 제안했습니다.

Comment thread packages/express-cargo/src/binding.ts Outdated
Comment on lines +353 to +355
if (cargo === undefined) {
throw new Error('Cargo not found on the request. Register the bindingCargo() middleware on this route before calling getCargo().')
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

req._cargo가 undefined뿐만 아니라 null일 가능성도 배제할 수 없습니다. 만약 null인 경우 cargo === undefined 조건문을 통과하여 null이 T 타입으로 캐스팅되어 반환되며, 이후 이 객체의 프로퍼티에 접근할 때 런타임 에러(TypeError)가 발생할 수 있습니다.

따라서 cargo === undefined 대신 cargo == null을 사용하여 더 안전하게 방어적 코딩을 적용하는 것이 좋습니다.

Suggested change
if (cargo === undefined) {
throw new Error('Cargo not found on the request. Register the bindingCargo() middleware on this route before calling getCargo().')
}
if (cargo == null) {
throw new Error('Cargo not found on the request. Register the bindingCargo() middleware on this route before calling getCargo().')
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_cargo 자체가 any 타입의 전략 확장 필드여서 외부에서 접근이 가능하고, 항상 유효한 T를 반환하고, 아니면 예외를 반환하기 위해 요청한 대로 null로 바꿔서 undefined, null 상황을 모두 방지하는 코드로 미리 작성.

@yuchem2 yuchem2 requested a review from laggu May 29, 2026 11:09
@yuchem2 yuchem2 merged commit 02ac845 into develop May 31, 2026
3 checks passed
@yuchem2 yuchem2 deleted the CARGO-419 branch May 31, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants