Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BE] fix: 회원가입 트랜잭션 범위 수정 & 타임아웃 시그널 수정 #257

Merged
merged 6 commits into from
Dec 14, 2023

Conversation

HKLeeeee
Copy link
Collaborator

@HKLeeeee HKLeeeee commented Dec 14, 2023

PR 설명

  • 회원가입 트랜잭션 범위 수정
  • 코틀린 타임아웃 메시지 미출력 수정

✅ 완료한 기능 명세

  • 회원가입 시 화원 저장 후 읽어오는 것까지 하나의 트랜잭션으로 처리
  • 트랜잭션 매니저 함수 내부에 logger가 오류를 일으켜 제거
  • README.md 수정
  • 타임아웃 시그널 수정

📸 스크린샷

고민과 해결과정

코틀린 코드가 타임아웃으로 종료될 때 메시지가 출력되지 않는 오류가 존재했음
-> 다른언어는 종료 시그널 SIGINT로 프로세스가 종료되었는데 코틀린은 종료코드 130으로 종료됨
SIGINT와 130은 동일한 POSIX 시그널!
따라서 이 프로세스가 어떤 시그널로 종료되었는지 체크하는 부분에 시그널과 코드 두 가지 모두 체크하게 함

@HKLeeeee HKLeeeee changed the title fix: [BE] 회원가입 트랜잭션 범위 수정 fix: [BE] 회원가입 트랜잭션 범위 수정 & 타임아웃 시그널 수정 Dec 14, 2023
@HKLeeeee HKLeeeee self-assigned this Dec 14, 2023
@HKLeeeee HKLeeeee added BackEnd 백엔드 관련 fix 오류 수정 labels Dec 14, 2023
@HKLeeeee HKLeeeee changed the title fix: [BE] 회원가입 트랜잭션 범위 수정 & 타임아웃 시그널 수정 [BE] fix: 회원가입 트랜잭션 범위 수정 & 타임아웃 시그널 수정 Dec 14, 2023
@HKLeeeee HKLeeeee requested a review from Gseungmin December 14, 2023 03:56
@@ -25,7 +25,8 @@ export class CodesService {
process.env.NODE_ENV === 'dev'
? path.join(__dirname, '..', 'tmp')
: '/algoitni';
private killSignal: NodeJS.Signals = 'SIGINT';
private readonly killSignal: NodeJS.Signals = 'SIGINT';
Copy link
Member

Choose a reason for hiding this comment

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

상수들 utils 파일에 보관하는 것도 좋아보여요!

@Gseungmin
Copy link
Member

LGTM!

@Gseungmin Gseungmin merged commit 918b652 into boostcampwm2023:dev Dec 14, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BackEnd 백엔드 관련 fix 오류 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants