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.

Sign up for GitHub

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

feat(simulator): integrate Sentry error tracking (#4962) #391

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(simulator): integrate Sentry error tracking (#4962) #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ed81334
feat: add sentry configuration to main.ts
AdityaP700 Nov 21, 2024
6a07ec6
feat: integrate Sentry error tracking for Simulator
AdityaP700 Nov 25, 2024
8b1a093
feat(simulator): integrate Sentry error tracking
AdityaP700 Nov 25, 2024
fc0271d
Merge branch 'main' into feat/sentry-integration-4962
AdityaP700 Nov 25, 2024
e7e2e1c
fix: address security and configuration issues
AdityaP700 Nov 25, 2024
fc69b13
Merge branch 'feat/sentry-integration-4962' of https://github.com/Adi…
AdityaP700 Nov 25, 2024
483b947
feat: add TypeScript types and env validation
AdityaP700 Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .env
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CURRENT_UID=1000
CURRENT_GID=1000
OPERATING_SYSTEM=windows
NON_ROOT_USERNAME=circuitverse
NON_ROOT_GROUPNAME=circuitverse
SENTRY_DSN=https://20a3411a988862503af74d4d8e7ec450@o4508321713684480.ingest.us.sentry.io/4508321717747712
6 changes: 6 additions & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Sentry Configuration
SENTRY_AUTH_TOKEN=
VITE_SENTRY_DSN=

# Environment
NODE_ENV=development
1,016 changes: 910 additions & 106 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
"dependencies": {
"@fortawesome/fontawesome-free": "^6.5.1",
"@mdi/font": "5.9.55",
"@sentry/tracing": "^8.40.0",
"@sentry/vue": "^8.40.0",
"@tiptap/core": "^2.0.3",
"@tiptap/extension-character-count": "^2.0.3",
"@tiptap/extension-subscript": "^2.0.3",
Expand Down Expand Up @@ -44,6 +46,8 @@
"@commitlint/cli": "^17.0.2",
"@commitlint/config-conventional": "^17.0.2",
"@intlify/vite-plugin-vue-i18n": "^3.4.0",
"@sentry/cli": "^2.39.0",
"@sentry/vite-plugin": "^2.22.6",
"@types/jquery": "^3.5.14",
"@types/webfontloader": "^1.0.0",
"@vitejs/plugin-vue": "^2.3.3",
Expand Down
5 changes: 5 additions & 0 deletions sentry.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
defaults.url=https://sentry.io/
defaults.org=circuitverse
defaults.project=javascript-vue
auth.token=${SENTRY_AUTH_TOKEN}
cli.executable=node_modules/@sentry/cli/bin/sentry-cli
32 changes: 29 additions & 3 deletions src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import router from './router/index'
import { createPinia } from 'pinia'
import { loadFonts } from './plugins/webfontloader'
import i18n from './locales/i18n'

import * as Sentry from "@sentry/vue"
import 'bootstrap'

import './globalVariables'

import './styles/css/main.stylesheet.css'
import '../node_modules/bootstrap/scss/bootstrap.scss'
import './styles/color_theme.scss'
Expand All @@ -20,9 +19,36 @@ import '@fortawesome/fontawesome-free/css/all.css'
loadFonts()

const app = createApp(App)
const isProd = import.meta.env.MODE === 'production'

Sentry.init({
app,
dsn: "https://20a3411a988862503af74d4d8e7ec450@o4508321713684480.ingest.us.sentry.io/4508321717747712",
integrations: [],
tracesSampleRate: isProd ? 0.2 : 1.0,
replaysSessionSampleRate: isProd ? 0.1 : 1.0,
replaysOnErrorSampleRate: 1.0,
trackComponents: true,
attachProps: true,
logErrors: true,
environment: import.meta.env.MODE,
release: import.meta.env.VITE_APP_VERSION || '1.0.0',
beforeSend(event) {
if (!isProd) {
console.error('Sentry error:', event);
return null;
}
return event;
},
tracingOptions: {
trackComponents: true,
timeout: 2000,
hooks: ['mount', 'update'],
},
});

app.use(createPinia())
app.use(vuetify)
app.use(router)
app.use(i18n)
app.mount('#app')
app.mount('#app')
45 changes: 31 additions & 14 deletions src/pages/simulator.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,44 @@
<template>
<Navbar />
<ContextMenu />
<Extra />
<Helper />
<div id="simulatorContainer">
<Navbar />
<ContextMenu />
<Extra />
<Helper />
<canvas
id="simulationArea"
ref="simulationCanvas"
class="simulation-canvas"
></canvas>
</div>
</template>

<script setup lang="ts">
import Navbar from '@/Navbar/Navbar.vue'
import ContextMenu from '@/ContextMenu/ContextMenu.vue'
import Extra from '@/Extra.vue'
import { defineComponent, onMounted } from 'vue'
import { defineComponent, onMounted, nextTick, ref } from 'vue'
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unused defineComponent import

The defineComponent import is not needed when using <script setup> syntax.

-import { defineComponent, onMounted, nextTick, ref } from 'vue'
+import { onMounted, nextTick, ref } from 'vue'
📝 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
import { defineComponent, onMounted, nextTick, ref } from 'vue'
import { onMounted, nextTick, ref } from 'vue'
🧰 Tools
🪛 eslint

[error] 19-19: 'defineComponent' is defined but never used.

(no-unused-vars)

import { setup as setupSimulator } from '../simulator/src/setup'
import Helper from '#/components/helpers/Helper.vue'

defineComponent({
components: {
Navbar,
ContextMenu,
Extra,
},
})
const simulationCanvas = ref<HTMLCanvasElement | null>(null)

onMounted(() => {
setupSimulator()
onMounted(async () => {
await nextTick()
if (simulationCanvas.value) {
const ctx = simulationCanvas.value.getContext('2d')
if (ctx) {
setupSimulator()
}
}
Comment on lines +25 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add error handling for canvas initialization with Sentry tracking

The canvas initialization code needs error handling as Sentry is available but not utilized. Add try-catch blocks around the canvas operations:

  • src/pages/simulator.vue: Add error handling for both canvas context acquisition and simulator setup:
onMounted(async () => {
    await nextTick()
    try {
        if (!simulationCanvas.value) {
            throw new Error('Canvas element not found');
        }
        const ctx = simulationCanvas.value.getContext('2d')
        if (!ctx) {
            throw new Error('Failed to get canvas context');
        }
        setupSimulator()
    } catch (error) {
        Sentry.captureException(error);
        // Handle UI feedback here
    }
})
🔗 Analysis chain

Verify Sentry error capture for canvas initialization failures

Since this is part of the Sentry integration PR, let's verify that initialization failures are properly captured.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if Sentry initialization and error handling are properly set up

# Look for Sentry.captureException or similar error handling
rg -l "Sentry\.(captureException|captureError)" 

# Check for any existing error boundary components
fd -e vue -e ts "ErrorBoundary"

# Look for other canvas initialization error handling
ast-grep --pattern 'catch (error) {
  $$$
}'

Length of output: 128

Script:

#!/bin/bash
# Let's check for Sentry initialization and error handling with broader patterns

# Check for Sentry imports and setup
rg -l "import.*Sentry" 

# Look for any error handling patterns in simulator.vue
rg -A 5 "catch|error" src/pages/simulator.vue

# Check for any try-catch blocks around canvas operations
rg -A 5 "getContext|setupSimulator" src/pages/simulator.vue

# Look for any error tracking or monitoring setup
rg -l "errorHandler|captureError|reportError"

Length of output: 540

Script:

#!/bin/bash
# Let's check the Sentry setup in main.ts and look for any error handling in the simulator setup

# Check Sentry initialization in main.ts
rg -A 5 "import.*Sentry" src/main.ts

# Look into simulator setup implementation
rg -A 10 "export.*setup" src/simulator/src/setup.ts

# Check for any error handling in the simulator module
rg "try|catch|throw" src/simulator/src/

Length of output: 3061

})
</script>

<style scoped>
.simulation-canvas {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
}
</style>
54 changes: 28 additions & 26 deletions src/simulator/src/simulationArea.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SimulationArea } from './interface/simulationArea'
import { clockTick } from './utils'

const simulationArea: SimulationArea = {
canvas: document.getElementById('simulationArea') as HTMLCanvasElement,
canvas: null,
context: null,
selected: false,
hover: false,
Expand All @@ -29,42 +29,44 @@ const simulationArea: SimulationArea = {
mouseDownX: 0,
mouseDownY: 0,
simulationQueue: new EventQueue(10000),
clickCount: 0,
lock: 'unlocked',
mouseDown: false,
ClockInterval: null,
touch: false,

timer() {
const clickTimer = setTimeout(() => {
simulationArea.clickCount = 0
}, 600)
if (this.clockEnabled) {
clockTick();
}
setTimeout(() => this.timer(), this.timePeriod);
},

setup() {
this.canvas = document.getElementById('simulationArea') as HTMLCanvasElement;
this.canvas.width = width;
this.canvas.height = height;
this.simulationQueue = new EventQueue(10000);
this.context = this.canvas.getContext('2d')!;
simulationArea.changeClockTime(simulationArea.timePeriod);
this.mouseDown = false;
},
changeClockTime(t: number) {
if (t < 50) {
if (!this.canvas) {
console.error('Simulation canvas not found');
return;
}
if (simulationArea.ClockInterval != null) {
clearInterval(simulationArea.ClockInterval);
}
simulationArea.timePeriod = t;
simulationArea.ClockInterval = setInterval(clockTick, t);
},
clear() {

this.context = this.canvas.getContext('2d');
if (!this.context) {
console.error('Could not get 2D context');
return;
}
this.context.clearRect(0, 0, this.canvas.width, this.canvas.height);

this.canvas.width = window.innerWidth;
this.canvas.height = window.innerHeight;
this.timer();
},

changeClockTime(t: number) {
if (t < 50) return;
this.timePeriod = t;
this.clockEnabled = true;
},

clear() {
if (this.context && this.canvas) {
this.context.clearRect(0, 0, this.canvas.width, this.canvas.height);
}
}
};

export { simulationArea }
export const { changeClockTime } = simulationArea
26 changes: 26 additions & 0 deletions src/types/env.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/// <reference types="vite/client" />

declare module '*.vue' {
import type { DefineComponent } from 'vue'
const component: DefineComponent<{}, {}, any>
export default component
}

interface ImportMetaEnv {
readonly VITE_SENTRY_DSN: string
readonly NODE_ENV: 'development' | 'production' | 'test'
}
Comment on lines +9 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Environment variables for Sentry configuration need to be properly utilized

The codebase shows hardcoded Sentry DSN in src/main.ts instead of using the defined VITE_SENTRY_DSN environment variable. Additionally, while SENTRY_AUTH_TOKEN is properly defined for build configuration, the runtime configuration could benefit from additional environment variables for:

  • tracesSampleRate
  • replaysSessionSampleRate

These values are currently hardcoded but would be better managed through environment variables for different environments.

  • src/main.ts: Replace hardcoded DSN with import.meta.env.VITE_SENTRY_DSN
  • src/types/env.d.ts: Consider adding: 
    readonly VITE_SENTRY_TRACES_SAMPLE_RATE?: string
readonly VITE_SENTRY_REPLAYS_SAMPLE_RATE?: string
🔗 Analysis chain

Verify completeness of Sentry environment variables.

Consider adding other common Sentry configuration variables that might be needed:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Sentry-related environment variable usage in the codebase
rg -g '!*.d.ts' 'VITE_SENTRY|SENTRY_' --type ts --type vue

# Check .env files for any additional Sentry variables
fd -e .env -x cat {} | grep -i 'sentry'

Length of output: 130

Script:

#!/bin/bash
# Search for Sentry-related environment variable usage with correct file types
rg 'VITE_SENTRY|SENTRY_' --type typescript --type-add 'vue:*.vue'

# Look for any .env files in the repository
fd '.env'

# Look for Sentry configuration or initialization code
rg 'Sentry\.(init|configure)' --type typescript --type-add 'vue:*.vue'

Length of output: 220

Script:

#!/bin/bash
# Search for Sentry-related environment variable usage with simpler approach
rg 'VITE_SENTRY|SENTRY_' -t ts -t tsx

# Look for any .env files in the repository
find . -name ".env*"

# Look for Sentry configuration or initialization code
rg 'Sentry\.(init|configure)' -t ts -t tsx

Length of output: 185

Script:

#!/bin/bash
# Check contents of .env files for Sentry configuration
cat .env.example

# Search for Sentry-related code without file type restrictions
rg 'VITE_SENTRY|SENTRY_'

# Search for Sentry initialization without file type restrictions
rg 'Sentry\.(init|configure)'

Length of output: 672

Script:

#!/bin/bash
# Check Sentry initialization configuration
rg -A 5 'Sentry\.init' src/main.ts

# Look for any other potential Sentry environment variables in the configuration
rg 'Sentry' src/main.ts

Length of output: 392


interface ImportMeta {
readonly env: ImportMetaEnv
}

declare global {
namespace NodeJS {
interface ProcessEnv {
SENTRY_AUTH_TOKEN: string
SENTRY_DSN: string
NODE_ENV: 'development' | 'production' | 'test'
}
}
}
13 changes: 13 additions & 0 deletions vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'
import { fileURLToPath, URL } from 'url'
import vueI18n from '@intlify/vite-plugin-vue-i18n'
import { sentryVitePlugin } from "@sentry/vite-plugin"

// https://github.com/vuetifyjs/vuetify-loader/tree/next/packages/vite-plugin
import vuetify from 'vite-plugin-vuetify'
Expand All @@ -23,6 +24,17 @@ export default defineConfig(() => ({
new URL('./src/locales/**', import.meta.url)
),
}),
sentryVitePlugin({
org: "circuitverse",
project: "javascript-vue",
authToken: process.env.SENTRY_AUTH_TOKEN,
...(!process.env.SENTRY_AUTH_TOKEN || !process.env.SENTRY_DSN) && {
throw new Error('SENTRY_AUTH_TOKEN and SENTRY_DSN are required for Sentry integration')
},
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the environment variable validation syntax

The current implementation has a syntax error in the spread operator. The error throwing logic needs to be restructured.

Apply this fix:

-          ...(!process.env.SENTRY_AUTH_TOKEN || !process.env.SENTRY_DSN) && {
-            throw new Error('SENTRY_AUTH_TOKEN and SENTRY_DSN are required for Sentry integration')
-          },
+          // Validate environment variables before plugin initialization
+          beforeBuild: () => {
+            if (!process.env.SENTRY_AUTH_TOKEN || !process.env.SENTRY_DSN) {
+              throw new Error('SENTRY_AUTH_TOKEN and SENTRY_DSN are required for Sentry integration');
+            }
+          },
📝 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
...(!process.env.SENTRY_AUTH_TOKEN || !process.env.SENTRY_DSN) && {
throw new Error('SENTRY_AUTH_TOKEN and SENTRY_DSN are required for Sentry integration')
},
// Validate environment variables before plugin initialization
beforeBuild: () => {
if (!process.env.SENTRY_AUTH_TOKEN || !process.env.SENTRY_DSN) {
throw new Error('SENTRY_AUTH_TOKEN and SENTRY_DSN are required for Sentry integration');
}
},
🧰 Tools
🪛 Biome (1.9.4)

[error] 32-32: expected : but instead found new

Remove new

(parse)

enabled: process.env.NODE_ENV === 'production',
telemetry: false,
debug: process.env.NODE_ENV === 'development'
}),
Comment on lines +27 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Sentry integration needs error handling improvements

The Sentry SDK is properly initialized in main.ts, but the implementation is missing crucial error handling components:

  • No global error handler is configured for Vue application
  • Missing error boundary components for graceful error handling
  • No custom error tracking setup for critical user interactions

To improve the integration:

  • Add app.config.errorHandler in main.ts to capture unhandled Vue errors
  • Implement error boundary components for critical UI sections
  • Consider adding errorCaptured hooks in top-level components
🔗 Analysis chain

Verify Sentry integration with the frontend application

The Sentry configuration looks good, but let's verify the integration with the frontend application.

Also applies to: 50-50

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Sentry integration in the frontend application

# Check for Sentry initialization in main application file
echo "Checking for Sentry initialization..."
rg -l "Sentry.init" src/

# Check for proper error boundaries in Vue components
echo "Checking for error boundary components..."
rg -l "errorCaptured|onErrorCaptured" src/

# Verify Sentry is imported in key application files
echo "Checking Sentry imports..."
rg -l "@sentry/(vue|browser)" src/

Length of output: 371

Script:

#!/bin/bash
# Let's check the actual implementation in main.ts and look for any error boundaries
echo "=== Checking Sentry initialization implementation ==="
rg -A 10 "Sentry.init" src/main.ts

echo -e "\n=== Checking for error boundary components or handlers ==="
# Looking for Vue error handlers with context
rg -A 5 "errorCaptured|onErrorCaptured|errorHandler" src/

echo -e "\n=== Checking app configuration for error handling ==="
# Looking for app-level error configuration
rg -A 5 "app\.(config|use)" src/main.ts

Length of output: 967

🧰 Tools
🪛 Biome (1.9.4)

[error] 32-32: expected : but instead found new

Remove new

(parse)

],
resolve: {
alias: {
Expand All @@ -35,6 +47,7 @@ export default defineConfig(() => ({
outDir: '../public/simulatorvue',
assetsDir: 'assets',
chunkSizeWarningLimit: 1600,
sourcemap: process.env.NODE_ENV === 'development', // Enable source maps only in development
},
test:{
globals: true,
Expand Down