-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor/#511 프론트의 토큰 만료 검증로직을 삭제하고 Axios를 도입한다. #554
Changes from 1 commit
3ec2b5f
895be60
69605c1
89bdb5f
e10bfbe
f2c79ee
3ecd234
db04ff0
a2bbcee
c287cfe
d06887a
a4a83df
4f858e9
09cf9f3
e6bfcfe
744fbff
2166a72
8e0a22f
0fbcfde
6755702
0f87b75
dbf9655
500cae1
ea5e89b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
/* eslint-disable prefer-const */ | ||
import axios from 'axios'; | ||
import { postRefreshAccessToken } from '@/features/auth/remotes/auth'; | ||
import type { AccessTokenRes } from '@/features/auth/types/auth.type'; | ||
import type { AxiosError, AxiosRequestConfig, InternalAxiosRequestConfig } from 'axios'; | ||
|
||
const { BASE_URL } = process.env; | ||
|
@@ -12,8 +14,10 @@ const defaultConfig: AxiosRequestConfig = { | |
withCredentials: true, | ||
}; | ||
|
||
export const clientBasic = axios.create(defaultConfig); | ||
export const client = axios.create(defaultConfig); | ||
const clientBasic = axios.create(defaultConfig); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 클라이언트 베이식은 인터셉터를 달지 않은 인스턴스 입니다. 리프레쉬 요청 함수에도 인터셉터를 부착한 인스턴스를 사용하는 경우, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 제가 이해한 게 맞나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
에러 인터셉터가 달려있는 인스턴스로 리패치 요청 + 이 요청에서 에러가 발생하면 다시 인터셉터에 걸림 + 또 요청 (다른방향으로 해결방법을 생각해보면, 카운터 변수를 하나 만들어서 리패치 요청의 횟수를 제한해도 되겠네용) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지금도 괜찮고, 다른 방향으로 제시한 것도 괜찮네요! 역시 도우밥! |
||
const client = axios.create(defaultConfig); | ||
|
||
let reissuePromise: Promise<AccessTokenRes> | null = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reissue 인터셉터 따로 모듈 분리하는 게 좋을 것 같아요. reissue에만 사용되는 변수인데 핵심 로직과 떨어져 있어서 계속 앞뒤로 왔다갔다하면서 읽게 되네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
좋습니당~ 공감하는 부분이라 사용처와 가깝게 let 변수의 line을 변경했어용!(반영 완) 💬 다만, 인터셉터는 인스턴스에 부착하는 로직인 만큼 한 파일에 모아두는게 더 좋다고 생각하는데, 어떻게 생각하세용? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그냥 제 생각을 말할게요! ( 이렇게 해달라는 것은 아닙니다! 도밥이 생각하고 정리하면 좋을 것 같아요! ) 별개로 만약 도밥이하나의 역할이라 결정 지었다면, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 좋은 접근법이네요. |
||
|
||
// 요청 인터셉터 | ||
const setToken = (config: InternalAxiosRequestConfig) => { | ||
|
@@ -26,27 +30,47 @@ const setToken = (config: InternalAxiosRequestConfig) => { | |
return config; | ||
}; | ||
|
||
// 응답 인터셉터 | ||
const refreshAccessTokenOnAuthError = async (error: AxiosError) => { | ||
// 응답 에러 인터셉터 | ||
const reissueOnExpiredTokenError = async (error: AxiosError) => { | ||
const originalRequest = error.config; | ||
const isAuthError = error.response?.status === 401; | ||
const hasAuthorization = !!originalRequest?.headers.Authorization; | ||
|
||
if (error.response?.status === 401 && originalRequest?.headers.Authorization) { | ||
if (isAuthError && hasAuthorization) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 401 에러이면서, 토큰을 가지고 있었을 때, 즉 사용자의 토큰이 만료되었을 때를 특정합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 401 에러 발생 시, 토큰을 가지고 있는 상황이라면 '토큰이 만료되었을 가능성'이 있다. 그러니 헤더에 토큰값이 있다면 재발급 요청을 보내라는 것이군요! 👍 |
||
try { | ||
const { accessToken } = await postRefreshAccessToken(); | ||
const { accessToken } = await (reissuePromise ??= reissue()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 프라미스의 특성을 이용한 로직입니다. 위 로직으로 조금이라도 먼저 호출된 api가 최초 1회만 reissue 요청을 보내고 뒤이어 실행되는 다수의 api는 해당 프라미스의 이행을 기다리게 됩니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reissue는 한번만 하는 로직을 만들기 어려웠을텐데 고생 많으셨네요! 하나의 프로미스로 관리한다는 점 인상 깊었습니다. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 예제를 만들어보니 이해가 되었네요. let promise = null;
const reissue = () => {
return new Promise((resolve) =>
setTimeout(() => {
console.log('promise');
resolve('accessToken');
}, 5000)
);
};
const onExpired = async () => {
const accessToken = await (promise ??= reissue());
console.log(accessToken);
};
onExpired();
onExpired();
onExpired();
onExpired();
// reissue promise는 한 번만 이행되지만
// console.log(accessToken)은 여러번 실행되어서
// promise
// accessToken *4 |
||
|
||
localStorage.setItem('userToken', accessToken); | ||
originalRequest.headers.Authorization = `Bearer ${accessToken}`; | ||
|
||
return client(originalRequest); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분 설명해주실 수 있나요? 본 요청을 다시 보내는 로직인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 맞아용. axios는 기존 config객체에 토큰 받아온걸 set하고, 다시 요청 보내는 로직입니당 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 와!우! 신기하네요! 정말 좋군요 axios~ |
||
} catch { | ||
window.alert('세션이 만료되었습니다. 다시 로그인 해주세요'); | ||
window.location.href = '/login'; | ||
} catch (error) { | ||
return Promise.reject(error); | ||
} | ||
} | ||
|
||
return Promise.reject(error); | ||
}; | ||
|
||
const reissue = async () => { | ||
try { | ||
const response = await postRefreshAccessToken(); | ||
const { accessToken } = response; | ||
|
||
localStorage.setItem('userToken', accessToken); | ||
return response; | ||
} catch (error) { | ||
window.alert('세션이 만료되었습니다. 다시 로그인 해주세요'); | ||
localStorage.removeItem('userToken'); | ||
window.location.href = '/login'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reissue에서 에러를 throw하고 리액트 단에서 라우팅해야할 것 같아요! 이거는 예외처리 (에러 바운더리)하면서 고려해보죠! 그리고 alert 부분도 로직과 ui가 결합되어 있는데 이것도 예외처리하면서 풀어낼 수 있을 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 맞아용. 저도 에러바운더리에서 해결하는 방법도 있다고 생각했어용. 에바 얼른 합시당 ! 으아아아아아아악! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ㅎㅎ 에바 고우고우 |
||
|
||
throw error; | ||
} finally { | ||
reissuePromise = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 reissuePromise를 응답 에러 인터셉터에서만 관리하는 것이 어떠한가요? 두 함수 모두 해당 변수를 조작할 필요는 없을 것 같다는 생각이 드네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 논리적으로 |
||
} | ||
}; | ||
|
||
clientBasic.interceptors.request.use(setToken); | ||
client.interceptors.request.use(setToken); | ||
client.interceptors.response.use((response) => response, refreshAccessTokenOnAuthError); | ||
client.interceptors.response.use((response) => response, reissueOnExpiredTokenError); | ||
|
||
export { clientBasic, client }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬
prefer-const
는error
가 아니라warning
인데 disable한 이유가 있으신가요There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
몰랐네요 ㅋㅋㅋㅋ 수정할게요~