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

[24.12.10 / TASK-44] Feature - 유저 이벤트 Tracking 기능 제작 #4

Merged
merged 95 commits into from
Dec 16, 2024

Conversation

six-standard
Copy link
Member

@six-standard six-standard commented Dec 10, 2024

유저 인터렉션 이벤트와 페이지 방문 이벤트를 Tracking하는 함수와 컴포넌트를 제작하였습니다.

trackUserEvent

유저가 페이지에서 한 행동을 Tracking할 때 사용합니다.
/track/user 경로로 POST 요청을 보냅니다.

TrackVIsitEvent

유저가 페이지에 접속한 시간, 페이지에서 나간 시간, 총 체류 시간(ms)을 Tracking할 때 사용합니다.
/track/visit 경로로 POST 요청을 보냅니다.

Summary by CodeRabbit

  • New Features

    • Sentry 통합을 위한 새로운 환경 변수 SENTRY_AUTH_TOKENSENTRY_DSN 추가.
    • Sentry를 위한 새로운 구성 파일(sentry.client.config.ts, sentry.edge.config.ts, sentry.server.config.ts) 생성.
    • 사용자 이벤트 추적을 위한 기능 추가(trackUserEvent, TrackVisitEvent).
    • 글로벌 오류 처리 컴포넌트 GlobalError 추가.
    • RootLayout 컴포넌트에 Sentry 오류 경계 추가.
  • Bug Fixes

    • API 오류 처리 개선 및 Sentry에 오류 로깅 추가.
  • Chores

    • .gitignore 파일에 Sentry 구성 파일 추가.
    • package.json에 Sentry 의존성 추가.
    • src/utils/index.ts에 이벤트 추적 기능을 재수출하는 코드 추가.
    • test-util.tsx 파일 삭제.

놀랍게도 현재 Next 15는 RC가 아니라는거~
+ 로그인 테스트 추가 (엣지케이스만)
괄호가 붙으면 URL로써 인식되지 않는데, 이 때문에 첫 페이지가 두개인 오류 발생
+ ToastContainer 추가
Copy link

coderabbitai bot commented Dec 10, 2024

Walkthrough

이 변경 사항은 Sentry 통합을 위한 여러 파일의 수정을 포함합니다. .env.sample 파일에 Sentry 인증 토큰을 추가하고, .gitignore에 Sentry 구성 파일을 추가하여 Git에서 무시하도록 설정했습니다. next.config.mjs 파일은 Sentry 설정을 통합하여 Next.js 구성에 추가하였으며, 새로운 Sentry 초기화 파일인 sentry.client.config.ts, sentry.edge.config.ts, sentry.server.config.ts를 생성했습니다. API 오류 처리를 개선하고, 글로벌 오류 처리 컴포넌트와 사용자 이벤트 추적 기능도 추가되었습니다.

Changes

파일 경로 변경 요약
.env.sample 새로운 환경 변수 SENTRY_AUTH_TOKEN, SENTRY_DSN 추가
.gitignore Sentry 구성 파일 .env.sentry-build-plugin 추가
next.config.mjs Sentry 통합을 위한 구성 추가, nextConfig 객체에 여러 Sentry 옵션 포함
package.json 새로운 의존성 @sentry/nextjs 추가
sentry.client.config.ts Sentry 클라이언트 초기화 구성 추가
sentry.edge.config.ts Sentry 엣지 기능 초기화 구성 추가
sentry.server.config.ts Sentry 서버 환경 초기화 구성 추가
src/api/index.ts Sentry를 통한 오류 추적을 위한 API 오류 처리 개선
src/app/global-error.tsx 글로벌 오류 상태를 처리하는 GlobalError 컴포넌트 추가
src/app/layout.tsx RootLayout에서 Sentry의 오류 경계를 추가하여 오류 관리 개선
src/instrumentation.ts 런타임 환경에 따른 Sentry 구성 관리 기능 추가
src/utils/eventTracker.tsx 사용자 이벤트 추적 기능 추가
src/utils/index.ts eventTracker 모듈의 모든 내보내기를 index.ts에서 재내보내기 추가
src/utils/test-util.tsx 테스트 유틸리티 함수가 포함된 파일 삭제

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • HA0N1
  • BDlhj
  • Nuung

🐇 변화의 기쁨을 노래해요,
Sentry와 함께 오류를 잡아내요.
새로운 파일과 설정이 더해져,
우리의 앱은 더욱 튼튼해져요!
오류의 바다를 헤치고 나가,
행복한 코드로 가득 채워요! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (3)
src/app/global-error.tsx (1)

12-14: 에러 복구 메커니즘 추가 필요

현재 구현은 에러를 Sentry에 보고만 하고 있습니다. 사용자 경험 향상을 위해 복구 옵션을 제공하는 것이 좋습니다.

다음과 같은 기능 추가를 제안합니다:

 useEffect(() => {
   Sentry.captureException(error);
-}, [error]);
+  
+  // 30초 후 자동으로 페이지 새로고침
+  const timer = setTimeout(() => {
+    window.location.reload();
+  }, 30000);
+  
+  return () => clearTimeout(timer);
+}, [error]);
src/utils/eventTracker.tsx (2)

45-53: useEffect 내의 이벤트 리스너 관리 개선이 필요합니다.

pathname이 변경될 때마다 방문 시간을 새로 측정해야 할 수 있습니다.

다음과 같이 수정하는 것을 추천드립니다:

-useEffect(() => {
+useEffect(() => {
   // 페이지 로드 시 시간 기록
   data.current.loadDate = new Date().toISOString();
+  data.current.path = pathname;  // pathname 업데이트
   window.addEventListener('unload', setUnloadData);

   return () => {
     window.removeEventListener('unload', setUnloadData);
   };
-}, []);
+}, [pathname]);

55-55: 접근성 개선이 필요합니다.

숨겨진 span 요소에 적절한 aria 속성을 추가하는 것이 좋습니다.

-return <span className="hidden">eventTracker</span>;
+return <span className="hidden" aria-hidden="true">eventTracker</span>;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1e6b9d1 and 16e8e40.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (13)
  • .env.sample (1 hunks)
  • .gitignore (1 hunks)
  • next.config.mjs (1 hunks)
  • package.json (1 hunks)
  • sentry.client.config.ts (1 hunks)
  • sentry.edge.config.ts (1 hunks)
  • sentry.server.config.ts (1 hunks)
  • src/api/index.ts (3 hunks)
  • src/app/global-error.tsx (1 hunks)
  • src/app/layout.tsx (2 hunks)
  • src/instrumentation.ts (1 hunks)
  • src/utils/eventTracker.tsx (1 hunks)
  • src/utils/index.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • .gitignore
  • src/utils/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/api/index.ts

[error] 65-67: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (13)
next.config.mjs (4)

4-4: 'reactStrictMode'가 false로 설정되어 있습니다

React의 Strict Mode는 잠재적인 문제를 감지하고 디버깅하는 데 도움이 됩니다. 'reactStrictMode'를 false로 설정하면 이러한 검사가 비활성화됩니다. 의도된 변경인지 확인해주세요.


20-21: 'widenClientFileUpload' 옵션은 빌드 시간을 증가시킬 수 있습니다

이 옵션은 더 자세한 스택 트레이스를 위해 더 많은 소스 맵을 업로드하지만, 그에 따라 빌드 시간이 늘어날 수 있습니다. 빌드 시간 증가가 허용 범위 내인지 확인해주세요.


28-32: 'tunnelRoute' 설정은 서버 부하를 증가시킬 수 있습니다

Sentry 요청을 리라이트하여 광고 차단기를 우회하면 서버 부하와 호스팅 비용이 증가할 수 있습니다. 이 설정이 필요한지 검토하고, 필요한 경우 서버 부하를 모니터링하는 것이 좋습니다.


40-44: 'automaticVercelMonitors'는 일부 환경에서 작동하지 않을 수 있습니다

이 옵션은 Vercel Cron Monitors의 자동 계측을 활성화하지만, 현재 App Router의 라우트 핸들러에서는 지원되지 않습니다. 이 기능이 현재 설정에서 필요한지 확인해주세요.

src/api/index.ts (3)

3-3: Sentry를 통한 에러 추적이 잘 통합되었습니다

에러 발생 시 Sentry를 통해 적절하게 에러가 캡처될 수 있도록 설정되었습니다.


26-26: 요청 헤더에 'Content-Type'이 설정되었습니다

'Content-Type' 헤더를 'application/json'으로 설정하여 JSON 형식의 데이터를 주고받을 수 있도록 구성되었습니다.


56-70: 에러 핸들링 로직이 개선되었습니다

에러 발생 시 상세한 정보를 Sentry에 전송하고, 적절한 커스텀 에러를 throw하도록 수정되었습니다.

🧰 Tools
🪛 Biome (1.9.4)

[error] 65-67: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

.env.sample (1)

3-3: SENTRY_AUTH_TOKEN 환경 변수가 추가되었습니다

Sentry 통합을 위해 필요한 인증 토큰 환경 변수가 추가되었습니다.

src/instrumentation.ts (2)

1-13: Sentry 초기화가 런타임에 따라 동적으로 처리됩니다

'NEXТ_RUNTIME' 환경 변수에 따라 서버 및 엣지 환경에서 적절한 Sentry 설정 파일을 로드하도록 구현되었습니다.


13-13: 'onRequestError'가 올바르게 설정되었습니다

Sentry의 'captureRequestError'를 사용하여 요청 에러를 캡처하도록 설정되었습니다.

src/app/layout.tsx (1)

4-4: Sentry ErrorBoundary 통합이 적절해 보입니다.

ErrorBoundary가 QueryProvider와 ToastContainer를 적절히 감싸고 있어 에러 추적이 가능합니다.

에러 처리가 제대로 작동하는지 확인하기 위해 다음 스크립트를 실행하세요:

Also applies to: 29-34

✅ Verification successful

The previous search didn't yield results, so I'm broadening the search to:

  1. Find ErrorBoundary usage
  2. Look for any Sentry method calls
  3. Examine the layout file directly to understand the implementation

This will help verify if error handling is properly implemented with Sentry.


Sentry 에러 처리가 올바르게 구현되었습니다

코드베이스 분석 결과, Sentry가 다음과 같이 적절하게 구현되어 있습니다:

  • ErrorBoundary가 전체 애플리케이션을 감싸고 있어 클라이언트 측 에러를 포착합니다
  • API 레이어에서 captureException을 사용하여 서버 에러를 추적합니다
  • Sentry 설정이 클라이언트, 서버, 엣지 환경 모두에 대해 올바르게 구성되어 있습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify error boundary implementation and related error handling

# Check for proper error handling setup
ast-grep --pattern 'catch($_) {
  $$$
  sentry.captureException($_)
  $$$
}'

Length of output: 78


Script:

#!/bin/bash
# Check for Sentry error boundary implementation
ast-grep --pattern 'ErrorBoundary$_'

# Check for error handling patterns in the codebase
rg "sentry\." -A 2

# Check layout.tsx content
cat src/app/layout.tsx

Length of output: 3937

sentry.client.config.ts (1)

21-21: 샘플링 비율 설정을 검토해주세요.

현재 설정된 샘플링 비율이 프로덕션 환경에 적합한지 확인이 필요합니다:

  • replaysSessionSampleRate: 0.1 (10%)
  • replaysOnErrorSampleRate: 1.0 (100%)

프로덕션 트래픽 규모에 따라 이 비율을 조정하는 것이 좋습니다. 특히 에러 발생 시 100% 캡처는 트래픽이 많은 경우 비용 증가를 초래할 수 있습니다.

Also applies to: 24-24

package.json (1)

15-15: Sentry 버전을 더 구체적으로 지정하는 것을 고려해 보세요

메이저 버전만 지정하는 것(^8)은 예기치 않은 호환성 문제를 일으킬 수 있습니다. 특정 마이너 버전으로 고정하는 것이 더 안전할 수 있습니다.

src/api/index.ts Outdated Show resolved Hide resolved
sentry.server.config.ts Outdated Show resolved Hide resolved
sentry.server.config.ts Outdated Show resolved Hide resolved
Comment on lines +16 to +26
return (
<html>
<body>
{/* `NextError` is the default Next.js error page component. Its type
definition requires a `statusCode` prop. However, since the App Router
does not expose status codes for errors, we simply pass 0 to render a
generic error message. */}
<NextError statusCode={0} />
</body>
</html>
);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

사용자 친화적인 에러 페이지 구현 필요

현재 구현은 기본적인 에러 페이지만 표시합니다. 사용자에게 더 유용한 정보와 액션을 제공하는 것이 좋습니다.

다음과 같은 개선을 제안합니다:

   return (
     <html>
       <body>
-        <NextError statusCode={0} />
+        <div className="error-container">
+          <h1>죄송합니다. 문제가 발생했습니다.</h1>
+          <p>잠시 후 다시 시도해 주세요.</p>
+          <button onClick={() => window.location.reload()}>
+            페이지 새로고침
+          </button>
+          {process.env.NODE_ENV !== 'production' && (
+            <pre>{error.message}</pre>
+          )}
+        </div>
       </body>
     </html>
   );

Committable suggestion skipped: line range outside the PR's diff.

sentry.edge.config.ts Outdated Show resolved Hide resolved
sentry.edge.config.ts Outdated Show resolved Hide resolved
sentry.client.config.ts Outdated Show resolved Hide resolved
Comment on lines 14 to 20
export const trackUserEvent = (message: string) => {
const { pathname } = window.location;
instance('/track/user', {
body: { path: pathname, message },
method: 'POST',
});
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

trackUserEvent 함수의 에러 처리와 타입 안전성 개선이 필요합니다.

현재 구현에는 다음과 같은 개선이 필요합니다:

  1. API 호출 실패에 대한 에러 처리가 없습니다
  2. message 파라미터의 타입 제한이 없습니다

다음과 같이 수정하는 것을 추천드립니다:

-export const trackUserEvent = (message: string) => {
+type TrackEventMessage = 'button_click' | 'form_submit' | 'page_view';
+
+export const trackUserEvent = async (message: TrackEventMessage) => {
   const { pathname } = window.location;
-  instance('/track/user', {
-    body: { path: pathname, message },
-    method: 'POST',
-  });
+  try {
+    await instance('/track/user', {
+      body: { path: pathname, message },
+      method: 'POST',
+    });
+  } catch (error) {
+    console.error('Failed to track user event:', error);
+    throw error;
+  }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const trackUserEvent = (message: string) => {
const { pathname } = window.location;
instance('/track/user', {
body: { path: pathname, message },
method: 'POST',
});
};
type TrackEventMessage = 'button_click' | 'form_submit' | 'page_view';
export const trackUserEvent = async (message: TrackEventMessage) => {
const { pathname } = window.location;
try {
await instance('/track/user', {
body: { path: pathname, message },
method: 'POST',
});
} catch (error) {
console.error('Failed to track user event:', error);
throw error;
}
};

src/utils/eventTracker.tsx Outdated Show resolved Hide resolved
sentry.client.config.ts Outdated Show resolved Hide resolved
@six-standard six-standard requested a review from Nuung December 12, 2024 15:52
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
src/api/index.ts (2)

2-4: import 구문 정리 필요

관련된 import 구문들을 그룹화하여 가독성을 향상시킬 수 있습니다:

  • 외부 라이브러리 (return-fetch)
  • 내부 모듈 (@/errors)
  • Sentry 관련 모듈
import returnFetch, { FetchArgs } from 'return-fetch';
-
+// Internal imports
import { ServerNotRespondingError } from '@/errors';
-import * as sentry from '@sentry/nextjs';
+// Error tracking
+import * as sentry from '@sentry/nextjs';

Line range hint 27-36: 응답 처리 로직 보완 필요

JSON 파싱 과정에서 발생할 수 있는 예외 처리가 누락되어 있습니다. 또한 응답 본문이 없는 경우에 대한 처리를 개선할 수 있습니다.

다음과 같이 개선할 것을 제안합니다:

   headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
   interceptors: {
     response: async (response) => {
       if (!response.ok) {
         throw response;
       }
+      try {
+        const text = await response?.text();
+        return {
+          ...response,
+          body: text ? JSON.parse(text) : null,
+        };
+      } catch (error) {
+        sentry.captureException(error);
+        throw new Error('응답 데이터 파싱 중 오류가 발생했습니다.');
+      }
-      return {
-        ...response,
-        body: response?.text ? JSON.parse(await response?.text()) : {},
-      };
     },
   },
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16e8e40 and 862b377.

📒 Files selected for processing (7)
  • .env.sample (1 hunks)
  • sentry.client.config.ts (1 hunks)
  • sentry.edge.config.ts (1 hunks)
  • sentry.server.config.ts (1 hunks)
  • src/api/index.ts (3 hunks)
  • src/utils/eventTracker.tsx (1 hunks)
  • src/utils/test-util.tsx (0 hunks)
💤 Files with no reviewable changes (1)
  • src/utils/test-util.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
  • sentry.client.config.ts
  • sentry.edge.config.ts
  • sentry.server.config.ts
  • src/utils/eventTracker.tsx
🔇 Additional comments (1)
.env.sample (1)

3-4: Sentry 인증 정보 보안 관리 확인 필요

환경 변수에 민감한 인증 정보가 포함되어 있으므로 다음 사항을 확인해 주세요:

  • 실제 토큰 값이 .gitignore에 포함되어 있는지 확인
  • 프로덕션 환경에서 안전하게 관리되고 있는지 검증
  • 토큰 값이 실수로 버전 관리 시스템에 포함되지 않도록 주의
✅ Verification successful

Sentry 환경 변수 설정이 적절히 보호되고 있음을 확인했습니다

  • .env 파일이 .gitignore에 올바르게 등록되어 있습니다
  • 실제 환경 변수 값이 코드베이스에 직접 노출되어 있지 않습니다
  • Sentry 설정 파일들에서는 환경 변수를 안전하게 참조하고 있습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify .gitignore contains proper entries

# Check if .env files are properly ignored
rg "^\.env(\..+)?$" .gitignore

Length of output: 62


Script:

#!/bin/bash
# Check for any committed .env files in git history
git ls-files | rg '\.env'

# Check if there are any environment variables directly set in the codebase
rg -g '!{.env*,*.md}' 'SENTRY_(AUTH_TOKEN|DSN)' 

Length of output: 288

Comment on lines +57 to +74
const context = err as Response;
sentry.setContext('Request', {
path: context.url,
status: context.status,
});
if ((err as Error).name === 'TimeoutError') {
sentry.captureException(new ServerNotRespondingError());
throw new ServerNotRespondingError();
else {
if (!error || !(error && error[`${(err as Response).status}`]))
throw new Error(`서버에서 오류가 발생했습니다. (${err.name})`);
} else {
if (!error?.[`${(err as Response).status}`]) {
const serverError = new Error(
`서버에서 예기치 않은 오류가 발생했습니다. (${err.name})`,
);
sentry.captureException(serverError);
throw serverError;
}

sentry.captureException(error[`${(err as Response).status}`]);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

에러 처리 로직 개선 필요

현재 에러 처리 로직에 다음과 같은 개선이 필요합니다:

  1. 에러 타입 처리가 any를 사용하고 있어 타입 안정성이 떨어집니다
  2. 중첩된 조건문으로 인해 복잡도가 높습니다
  3. 에러 메시지가 하드코딩되어 있습니다

다음과 같이 개선할 것을 제안합니다:

-  } catch (err: any) {
+  } catch (err: unknown) {
     const context = err as Response;
     sentry.setContext('Request', {
       path: context.url,
       status: context.status,
     });
-    if ((err as Error).name === 'TimeoutError') {
+    
+    // TimeoutError 처리
+    if (err instanceof Error && err.name === 'TimeoutError') {
       sentry.captureException(new ServerNotRespondingError());
       throw new ServerNotRespondingError();
-    } else {
-      if (!error?.[`${(err as Response).status}`]) {
-        const serverError = new Error(
-          `서버에서 예기치 않은 오류가 발생했습니다. (${err.name})`,
-        );
-        sentry.captureException(serverError);
-        throw serverError;
-      }
+      return;
+    }
-      sentry.captureException(error[`${(err as Response).status}`]);
-      throw error[`${(err as Response).status}`];
+    // 상태 코드별 에러 처리
+    const statusCode = (err as Response).status;
+    const statusError = error?.[statusCode];
+    
+    if (!statusError) {
+      const serverError = new Error(
+        `서버에서 예기치 않은 오류가 발생했습니다. (${err instanceof Error ? err.name : 'Unknown Error'})`,
+      );
+      sentry.captureException(serverError);
+      throw serverError;
     }
+    
+    sentry.captureException(statusError);
+    throw statusError;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const context = err as Response;
sentry.setContext('Request', {
path: context.url,
status: context.status,
});
if ((err as Error).name === 'TimeoutError') {
sentry.captureException(new ServerNotRespondingError());
throw new ServerNotRespondingError();
else {
if (!error || !(error && error[`${(err as Response).status}`]))
throw new Error(`서버에서 오류가 발생했습니다. (${err.name})`);
} else {
if (!error?.[`${(err as Response).status}`]) {
const serverError = new Error(
`서버에서 예기치 않은 오류가 발생했습니다. (${err.name})`,
);
sentry.captureException(serverError);
throw serverError;
}
sentry.captureException(error[`${(err as Response).status}`]);
const context = err as Response;
sentry.setContext('Request', {
path: context.url,
status: context.status,
});
// TimeoutError 처리
if (err instanceof Error && err.name === 'TimeoutError') {
sentry.captureException(new ServerNotRespondingError());
throw new ServerNotRespondingError();
return;
}
// 상태 코드별 에러 처리
const statusCode = (err as Response).status;
const statusError = error?.[statusCode];
if (!statusError) {
const serverError = new Error(
`서버에서 예기치 않은 오류가 발생했습니다. (${err instanceof Error ? err.name : 'Unknown Error'})`,
);
sentry.captureException(serverError);
throw serverError;
}
sentry.captureException(statusError);
throw statusError;

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
src/utils/eventTracker.tsx (1)

25-62: 컴포넌트 문서화와 접근성 개선이 필요합니다.

컴포넌트의 목적과 동작 방식을 명확하게 설명하는 문서화가 필요합니다. 또한, 숨겨진 span 요소의 접근성을 개선할 수 있습니다.

다음과 같이 수정하는 것을 추천드립니다:

+/**
+ * 페이지 방문 시간을 추적하는 컴포넌트
+ * 
+ * @description
+ * - 페이지 로드 시점 기록
+ * - 페이지 언로드 시점 기록
+ * - 총 방문 시간 계산 및 서버로 전송
+ */
 export const TrackVisitEvent = () => {
   // ... existing code ...
 
-  return <span className="hidden">eventTracker</span>;
+  return <span className="hidden" aria-hidden="true">방문 시간 추적</span>;
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 862b377 and 22c47e5.

📒 Files selected for processing (1)
  • src/utils/eventTracker.tsx (1 hunks)
🔇 Additional comments (2)
src/utils/eventTracker.tsx (2)

17-23: ⚠️ Potential issue

에러 처리와 타입 안전성 개선이 여전히 필요합니다.

이전 리뷰에서 지적된 문제가 아직 해결되지 않았습니다:

  1. API 호출 실패에 대한 에러 처리가 없습니다
  2. event_type 파라미터가 더 명확한 타입으로 정의될 필요가 있습니다

다음과 같이 수정하는 것을 추천드립니다:

-export const trackUserEvent = (event_type: messages) => {
+export const trackUserEvent = async (event_type: EventType) => {
   const { pathname } = window.location;
-  instance('/event', {
-    body: { path: pathname, event_type },
-    method: 'POST',
-  });
+  try {
+    await instance('/event', {
+      body: { path: pathname, event_type },
+      method: 'POST',
+    });
+  } catch (error) {
+    console.error('이벤트 추적 실패:', error);
+    throw error;
+  }
 };

35-49: ⚠️ Potential issue

setUnloadData 함수의 데이터 처리 방식에 개선이 필요합니다.

현재 구현에서 발견된 기술적 문제점들:

  1. BigInt 사용이 불필요합니다 - 일반 number 타입으로 충분합니다
  2. data.current.toString()은 객체를 올바르게 직렬화하지 않습니다
  3. loadDate null 체크가 없습니다

다음과 같이 수정하는 것을 추천드립니다:

 const setUnloadData = () => {
   const date = new Date().toISOString();
+  if (!data.current.loadDate) {
+    console.error('로드 시간이 설정되지 않았습니다');
+    return;
+  }
+
   data.current = {
     ...data.current,
-    visitTime: BigInt(
+    visitTime: (
       new Date(date).getTime() -
-        new Date(data.current.loadDate as string).getTime(),
-    ).toString(),
+      new Date(data.current.loadDate).getTime()
+    ).toString(),
     unloadDate: date,
   };
+
+  const blob = new Blob(
+    [JSON.stringify(data.current)],
+    { type: 'application/json' }
+  );
+
   navigator.sendBeacon(
     `${process.env.NEXT_PUBLIC_BASE_URL}/stay`,
-    data.current.toString(),
+    blob
   );
 };

Likely invalid or redundant comment.

path: string;
};

type messages = '01' | '02' | '03' | '04' | '99';
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

이벤트 타입을 더 명확하게 정의해주세요.

현재 messages 타입이 숫자 문자열('01', '02' 등)을 사용하고 있어 각 이벤트가 무엇을 의미하는지 파악하기 어렵습니다.

다음과 같이 수정하는 것을 추천드립니다:

-type messages = '01' | '02' | '03' | '04' | '99';
+type EventType =
+  | 'page_view'        // '01'
+  | 'button_click'     // '02'
+  | 'form_submit'      // '03'
+  | 'search'           // '04'
+  | 'custom_event'     // '99'
+;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
type messages = '01' | '02' | '03' | '04' | '99';
type EventType =
| 'page_view' // '01'
| 'button_click' // '02'
| 'form_submit' // '03'
| 'search' // '04'
| 'custom_event' // '99'
;

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

PR 리뷰 어게인~ 고생많으셨어요, 잘 봤습니다 👍 👍

아래 2개의 실제 값은 어떻게 공유주실 예정일까요?!

SENTRY_AUTH_TOKEN=<'sentry auth token here'>
SENTRY_DSN=<'sentry dsn here'>

@six-standard
Copy link
Member Author

PR 리뷰 어게인~ 고생많으셨어요, 잘 봤습니다 👍 👍

아래 2개의 실제 값은 어떻게 공유주실 예정일까요?!

SENTRY_AUTH_TOKEN=<'sentry auth token here'> SENTRY_DSN=<'sentry dsn here'>

Slack의 공지 채널을 통해 공유드리면 될 듯 합니다!

Copy link

@HA0N1 HA0N1 left a comment

Choose a reason for hiding this comment

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

참고해서 tracking 완료하면 될 거 같네요 잘봤습니다!

좋았던 점

함수나 변수명이 명확해서 어떤 역할을 하는지 쉽게 확인할 수 있어 좋았습니다!


export const trackUserEvent = (event_type: messages) => {
const { pathname } = window.location;
instance('/event', {
Copy link

Choose a reason for hiding this comment

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

현재 노션에서 /api/track로 기억하는데 /event 로 보내실 예정인가요??

Copy link
Member Author

Choose a reason for hiding this comment

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

맞춰서 다시 작성해두겠습니다!

Copy link

@BDlhj BDlhj left a comment

Choose a reason for hiding this comment

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

PR 잘 봤습니다~~

좋았던 점

sendbeacon은 아예 처음 봤는데 덕분에 찾아보면서 fetch와 차이점을 알 수 있었네요! 안정성을 고려하신 부분이 좋았습니다!

@six-standard six-standard merged commit d6a6324 into main Dec 16, 2024
1 check passed
@Nuung Nuung deleted the feature/user-event branch December 22, 2024 09:29
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.

4 participants