feat: show real time probe logs on the probe page #122
Conversation
BREAKING CHANGE: GP_API_URL env variable now contains the API address, auth url is defined via the GP_AUTH_URL env variable
WalkthroughAdds a new Vue 3 script-setup TypeScript component TabLogs.vue (prop: probeId) that streams probe logs from runtimeConfig.public.gpApiUrl (/v1/probes/{probeId}/logs) using periodic useLazyAsyncData polling (2s), appends ProbeLog entries into a reactive array capped at 5000, and exposes Live tail toggling, autoscroll with user-suspend, loading indicators (LogLoader), and error toasts. Introduces global type ProbeLog, LogLoader.vue markup, tailwind dot-pulse keyframes/animation, a NuxtIcon component replacing nuxt-icon usages, public runtimeConfig gpApiUrl, the useProbeDetailTabs composable (tab sync via URL using "details" | "logs"), and several typing/non-null assertion and tsconfig changes. Possibly related PRs
Suggested labels
Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pages/probes/[id].vue (2)
99-99: Consider adding error boundaries for the tab components.The Tabs component should handle potential errors from child components gracefully, especially when loading dynamic content like logs.
110-112: Consider lazy loading the TabLogs component.Since the Logs tab might not be immediately accessed by all users, consider lazy loading the component to improve initial page load performance.
- <ProbeTabLogs :is-active="activeTab === '1'"/> + <LazyProbeTabLogs :is-active="activeTab === '1'"/>components/probe/TabLogs.vue (1)
67-74: Improve error recovery mechanism.The current error handling silently catches errors. Consider implementing a retry mechanism with exponential backoff for transient failures.
+const retryCount = ref(0); +const maxRetries = 3; + const refreshLogs = async () => { return refresh().then(() => { if (!error.value) { lastFetched.value = Date.now(); + retryCount.value = 0; // Reset retry count on success } - }).catch(() => {}) // errors are displayed via useErrorToast + }).catch((err) => { + // Implement exponential backoff for retries + if (retryCount.value < maxRetries) { + retryCount.value++; + const delay = Math.min(1000 * Math.pow(2, retryCount.value), 30000); + setTimeout(() => refreshLogs(), delay); + } + }) .finally(() => { showLoader.value = !isActive; }); // do not show the loader on further refetches };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
components/probe/TabLogs.vue(1 hunks)layouts/default.vue(1 hunks)nuxt.config.ts(1 hunks)pages/probes/[id].vue(3 hunks)types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
types.tspages/probes/[id].vuecomponents/probe/TabLogs.vuenuxt.config.tslayouts/default.vue
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/[id].vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (4)
layouts/default.vue (1)
204-204: LGTM!The addition of
h-fullclass ensures proper height inheritance for the flex-based tab layout in the probe details page.types.ts (1)
205-210: LGTM!The
ProbeLogtype definition is well-structured with appropriate optional fields for log metadata.nuxt.config.ts (1)
6-8: Verify environment variable migration for existing deploymentsPlease ensure that any places still relying on the old GP_API_URL for the authentication endpoint are updated to use the new GP_AUTH_URL, and that your docs and sample config files reflect this change.
• Check your environment-variable sample files (e.g. .env.example, .env.production, CI/CD pipeline settings) for any references to GP_API_URL as an auth URL and add or update entries for GP_AUTH_URL.
• Update README.md (or other documentation) to list GP_AUTH_URL alongside GP_API_URL, so that future deploys don’t miss the new variable.
• Verify that all staging/production/local deployment configurations now set GP_AUTH_URL correctly to avoid falling back to the public default (“https://auth.globalping.io”).components/probe/TabLogs.vue (1)
17-19: Remove unnecessary sanitization suggestionVue’s built-in moustache interpolation (
{{ … }}) already HTML-escapes all content, so using{{ log.message }}cannot introduce XSS even iflog.messagecontains malicious markup. Since you’re not usingv-htmlor any raw HTML insertion here, no additional sanitization is required.Likely an incorrect or invalid review comment.
|
Came up with a simple design, suggestions are welcome. @MartinKolarik |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/probe/TabLogs.vue (3)
74-74: Remove empty catch block for clearer error handlingThe empty catch block serves no purpose since errors are already handled by
useErrorToast. Consider removing it to make the error flow more explicit.const refreshLogs = async () => { return refresh().then(() => { if (!error.value) { lastFetched.value = Date.now(); } - }).catch(() => {}) // errors are displayed via useErrorToast - .finally(() => { showLoader.value = !isActive; }); // do not show the loader on further refetches + }).finally(() => { showLoader.value = !isActive; }); // do not show the loader on further refetches };
110-111: Add defensive check for data.value before spreadingWhile the default value ensures
data.valueis an array, adding a defensive check prevents potential runtime errors if the API response structure changes.// append new logs to the already stored ones watch(() => data.value?.length, () => { - logs.value.push(...data.value); + if (data.value) { + logs.value.push(...data.value); + }
19-19: Consider usingbreak-allinstead ofbreak-wordsfor log messagesThe
break-wordsclass is deprecated in favor ofbreak-allorbreak-normalin modern Tailwind CSS versions. For log messages that may contain long strings without spaces (like URLs or stack traces),break-allprovides better wrapping behavior.- <span class="break-words">{{ log.message }}</span> + <span class="break-all">{{ log.message }}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/probe/TabLogs.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
components/probe/TabLogs.vue
🧠 Learnings (8)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.276Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-08-21T17:21:19.276Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.276Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-14T20:25:06.380Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-21T17:23:35.404Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:108-111
Timestamp: 2025-08-21T17:23:35.404Z
Learning: useLazyFetch in Nuxt 3 automatically handles request deduplication by key (URL or explicit key). It prevents multiple concurrent identical requests using either 'cancel' (default - abort existing and start new) or 'defer' (wait for pending request to complete) strategies. This is request-level deduplication, not data-level deduplication.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-15T10:55:44.287Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:479-486
Timestamp: 2025-08-15T10:55:44.287Z
Learning: In Nuxt's useLazyAsyncData, the loading/pending state is not initially false when the composable hasn't started yet. The loading state behavior differs from what might be expected.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 doesn't cache data by default. It uses keys primarily to avoid fetch duplication, not for cache isolation. The default auto-generated key based on file path and line number is typically sufficient unless there's a specific need for custom key generation.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 uses keys primarily for deduplication (preventing duplicate fetches), not for cache isolation. Default auto-generated keys based on file path and line number are sufficient for most use cases. There's no persistent caching by default, so explicit keys are only needed when you want to share data across different components or calls, not to prevent "cache conflicts."
Applied to files:
components/probe/TabLogs.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run e2e tests
|
It would be nice to preserve logs tab view on page reload. |
|
Adoption code dogs are not feeling well 😄 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pages/probes/index.vue (1)
365-380: Avoid potential TypeError in credits transform; relax type to allow null.
data[0]?.sum.amountwill throw ifsumisundefined. Also, Directus aggregates may returnnullfor empty sums.Apply:
- () => $directus.request<{ sum: { amount: number } }[]>(aggregate('gp_credits_additions', { + () => $directus.request<{ sum: { amount: number | null } }[]>(aggregate('gp_credits_additions', { @@ - transform: data => data[0]?.sum.amount ?? 0, + transform: data => data[0]?.sum?.amount ?? 0,pages/probes/[id].vue (3)
80-88: Guard against undefined altIps.
probeDetails.altIps.lengthcan throw ifaltIpsis missing.- v-if="probeDetails.altIps.length > 1" + v-if="probeDetails?.altIps?.length > 1"
188-200: Avoid potential TypeError in credits transform; relax type to allow null.Same aggregate issue as on the list page.
- () => $directus.request<{ sum: { amount: number }; adopted_probe: string }[]>(aggregate('gp_credits_additions', { + () => $directus.request<{ sum: { amount: number | null }; adopted_probe: string }[]>(aggregate('gp_credits_additions', { @@ - { watch: [ probeDetails ], transform: data => data[0]?.sum.amount ?? 0 }, + { watch: [ probeDetails ], transform: data => data[0]?.sum?.amount ?? 0 },
191-194: Remove hardcoded 'admin' fallback in Directus filter.Fallbacking to
'admin'can leak/aggregate unrelated credits. Use the standardized user filter for consistency with index.vue.- github_id: { _eq: user.value.external_identifier || 'admin' }, + ...getUserFilter('github_id'), adopted_probe: { _eq: probeDetails?.value?.id },Add (outside the shown range; Nuxt auto-imports may already handle it if configured):
import { useUserFilter } from '~/composables/useUserFilter'; const { getUserFilter } = useUserFilter();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
pages/probes/[id].vue(5 hunks)pages/probes/index.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
pages/probes/[id].vuepages/probes/index.vue
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/[id].vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
|
show loader even after the first load (elsewhere) and add additional information |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/probe/TabLogs.vue (1)
104-112: Lower default poll interval to 2000msMatches prior feedback; improves “real-time” feel without notable overhead.
-const setRefreshInterval = (timeout = 10000) => { +const setRefreshInterval = (timeout = 2000) => {
🧹 Nitpick comments (3)
components/probe/TabLogs.vue (3)
53-53: Avoid Node types in browser code for interval refUse an env-agnostic type.
-const refreshInterval = ref<NodeJS.Timeout>(); +const refreshInterval = ref<ReturnType<typeof setInterval> | null>(null);
9-11: Use a stable key instead of array indexPrevents rendering glitches if logs ever re-order or de-dup happens.
- :key="index" + :key="log.timestamp ? `${log.timestamp}-${index}` : index"
48-48: Request only the last ~20 lines on first load (aligns with issue #26)Add an initial tail limit; adjust the param name to match the backend.
const MAX_LOGS = 5000; +const INITIAL_TAIL = 20;() => $fetch(`${config.public.gpApiUrl}/v1/probes/${probeId}/logs`, { params: { since: lastFetched.value, + // NOTE: verify backend param name (e.g., limit|tail) + limit: lastFetched.value === 0 ? INITIAL_TAIL : undefined, }, credentials: 'include', }),Please confirm the correct query param supported by the API before merging.
Also applies to: 60-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/probe/TabLogs.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
components/probe/TabLogs.vue
🧠 Learnings (8)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-08-21T17:21:19.320Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-14T20:25:06.380Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-21T17:23:35.449Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:108-111
Timestamp: 2025-08-21T17:23:35.449Z
Learning: useLazyFetch in Nuxt 3 automatically handles request deduplication by key (URL or explicit key). It prevents multiple concurrent identical requests using either 'cancel' (default - abort existing and start new) or 'defer' (wait for pending request to complete) strategies. This is request-level deduplication, not data-level deduplication.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-15T10:55:44.287Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:479-486
Timestamp: 2025-08-15T10:55:44.287Z
Learning: In Nuxt's useLazyAsyncData, the loading/pending state is not initially false when the composable hasn't started yet. The loading state behavior differs from what might be expected.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 doesn't cache data by default. It uses keys primarily to avoid fetch duplication, not for cache isolation. The default auto-generated key based on file path and line number is typically sufficient unless there's a specific need for custom key generation.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 uses keys primarily for deduplication (preventing duplicate fetches), not for cache isolation. Default auto-generated keys based on file path and line number are sufficient for most use cases. There's no persistent caching by default, so explicit keys are only needed when you want to share data across different components or calls, not to prevent "cache conflicts."
Applied to files:
components/probe/TabLogs.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/probes/[id].vue (1)
81-87: Guard against undefined altIpsAccessing .length without optional chaining can throw.
- <button - v-if="probeDetails.altIps.length > 1" + <button + v-if="probeDetails.altIps?.length > 1"
♻️ Duplicate comments (2)
components/probe/TabLogs.vue (1)
75-79: Avoid clock-skew gaps: advance since by server timestamps, not client timeUsing Date.now() can skip logs if the client clock is ahead. Move the cursor using the last item’s timestamp (only when items exist).
- return refresh().then(() => { - if (!error.value) { - lastFetched.value = Date.now(); - } - }).finally(() => { showLoader.value = false; }); // do not show the loader on further refetches + return refresh().then(() => { + if (!error.value) { + const items = data.value || []; + if (items.length) { + const last = items[items.length - 1]; + const ts = Number(new Date((last as any)?.timestamp ?? 0)); + if (!Number.isNaN(ts)) lastFetched.value = ts + 1; // avoid >= duplicates + } + } + }).finally(() => { showLoader.value = false; }); // do not show the loader on further refetchespages/probes/[id].vue (1)
101-112: Gate Logs by probe readiness and fix focusability when disabledRestrict per requirement and prevent focusing a disabled tab.
- <Tab value="details" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Details</Tab> - <Tab value="logs" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Logs</Tab> + <Tab value="details" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Details</Tab> + <Tab + value="logs" + :disabled="!isLogsEnabled" + :tabindex="isLogsEnabled ? 0 : -1" + class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto" + >Logs</Tab> @@ - <TabPanel class="flex min-h-0 flex-1 flex-col" value="logs" tabindex="-1"> - <ProbeTabLogs :probe-uuid="probeDetails.uuid"/> - </TabPanel> + <TabPanel v-if="isLogsEnabled" class="flex min-h-0 flex-1 flex-col" value="logs" tabindex="-1"> + <ProbeTabLogs :probe-uuid="probeDetails.uuid"/> + </TabPanel>Add in <script setup>:
+const isLogsEnabled = computed(() => probeDetails.value?.status === 'ready');
🧹 Nitpick comments (5)
composables/useProbeDetailTabs.ts (2)
11-19: Use router.replace and guard to avoid history bloat and redundant navigationsSwitch to replace and only mutate the URL when it actually changes to prevent back-button churn and unnecessary navigations.
-watch(activeTab, (val) => { - if (val === DEFAULT_TAB) { - const { tab, ...rest } = route.query; - router.push({ query: { ...rest } }); - return; - } - - router.push({ query: { ...route.query, tab: val } }); -}); +watch(activeTab, (val) => { + const { tab, ...rest } = route.query as Record<string, unknown>; + if (val === DEFAULT_TAB) { + if (tab !== undefined) router.replace({ query: { ...rest } }); + return; + } + if (route.query.tab !== val) { + router.replace({ query: { ...route.query, tab: val } }); + } +}); -watch(() => route.query.tab, (newTab) => { - if (typeof newTab === 'string' && TAB_NAMES.includes(newTab)) { - activeTab.value = newTab as TabName; - return; - } - - const { tab, ...rest } = route.query; - router.push({ query: { ...rest } }); - activeTab.value = DEFAULT_TAB; -}, { immediate: true }); +watch(() => route.query.tab, (newTab) => { + if (typeof newTab === 'string' && TAB_NAMES.includes(newTab as any)) { + if (activeTab.value !== newTab) activeTab.value = newTab as TabName; + return; + } + const { tab, ...rest } = route.query as Record<string, unknown>; + if (tab !== undefined) router.replace({ query: { ...rest } }); + if (activeTab.value !== DEFAULT_TAB) activeTab.value = DEFAULT_TAB; +}, { immediate: true });Also applies to: 21-30
5-8: Derive TabName from the constant to keep the union in syncThis prevents drift between the type and allowed values.
-type TabName = 'details' | 'logs'; -const TAB_NAMES = [ 'details', 'logs' ]; -const DEFAULT_TAB = 'details'; +const TAB_NAMES = ['details', 'logs'] as const; +type TabName = typeof TAB_NAMES[number]; +const DEFAULT_TAB: TabName = 'details';components/probe/TabLogs.vue (3)
52-52: Fix timer typing for browser buildsNodeJS.Timeout is not correct in the browser; prefer ReturnType.
-const refreshInterval = ref<NodeJS.Timeout>(); +const refreshInterval = ref<ReturnType<typeof setInterval>>();
8-12: Use a stable key for v-for itemsIndex keys can cause rendering glitches on insertions. Prefer a deterministic key.
- :key="index" + :key="`${log.timestamp ?? ''}|${log.level ?? ''}|${log.scope ?? ''}|${index}`"
127-131: Cancel the debounced handler on unmountAvoid retaining the debounced callback after teardown.
onUnmounted(() => { if (refreshInterval.value) { clearInterval(refreshInterval.value); } + // lodash debounce exposes cancel() + // @ts-expect-error runtime method exists + onScrollDebounced.cancel?.(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/probe/TabLogs.vue(1 hunks)composables/useProbeDetailTabs.ts(1 hunks)pages/probes/[id].vue(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
composables/useProbeDetailTabs.tscomponents/probe/TabLogs.vuepages/probes/[id].vue
🧠 Learnings (9)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-08-21T17:21:19.320Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
components/probe/TabLogs.vuepages/probes/[id].vue
📚 Learning: 2025-08-14T20:25:06.380Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
components/probe/TabLogs.vuepages/probes/[id].vue
📚 Learning: 2025-08-21T17:23:35.449Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:108-111
Timestamp: 2025-08-21T17:23:35.449Z
Learning: useLazyFetch in Nuxt 3 automatically handles request deduplication by key (URL or explicit key). It prevents multiple concurrent identical requests using either 'cancel' (default - abort existing and start new) or 'defer' (wait for pending request to complete) strategies. This is request-level deduplication, not data-level deduplication.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-15T10:55:44.287Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:479-486
Timestamp: 2025-08-15T10:55:44.287Z
Learning: In Nuxt's useLazyAsyncData, the loading/pending state is not initially false when the composable hasn't started yet. The loading state behavior differs from what might be expected.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 doesn't cache data by default. It uses keys primarily to avoid fetch duplication, not for cache isolation. The default auto-generated key based on file path and line number is typically sufficient unless there's a specific need for custom key generation.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 uses keys primarily for deduplication (preventing duplicate fetches), not for cache isolation. Default auto-generated keys based on file path and line number are sufficient for most use cases. There's no persistent caching by default, so explicit keys are only needed when you want to share data across different components or calls, not to prevent "cache conflicts."
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-04-29T21:58:13.738Z
Learnt from: xbpcb
PR: jsdelivr/globalping-dash#72
File: pages/probes/[id].vue:511-519
Timestamp: 2025-04-29T21:58:13.738Z
Learning: In the GlobalPing Dashboard application, components fully unmount and remount when navigating between different IDs in dynamic routes (e.g., from `/probes/123` to `/probes/456`), so explicit watchers for route parameter changes are not required.
Applied to files:
pages/probes/[id].vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (1)
pages/probes/[id].vue (1)
99-112: Confirm deviation from the “modal” UX in the issueThe issue requests a modal; this implements a tab. If that’s intentional, acknowledge in the issue or update acceptance criteria.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/probe/LogLoader.vue (1)
2-5: Fix non-existent animate-delay utilities; add a11y + reduced-motion support
animate-delay-300andanimate-delay-[600ms]aren’t Tailwind core utilities and will be ignored unless you’ve added a plugin. Also, the loader lacks accessible semantics and a reduced-motion fallback.Apply:
- <span class="flex flex-nowrap gap-1"> - <span class="size-1.5 animate-dot-pulse rounded-full bg-gray-500 opacity-50"/> - <span class="size-1.5 animate-dot-pulse rounded-full bg-gray-500 opacity-50 animate-delay-300"/> - <span class="size-1.5 animate-dot-pulse rounded-full bg-gray-500 opacity-50 animate-delay-[600ms]"/> + <span class="flex flex-nowrap gap-1" role="status" aria-live="polite"> + <span class="sr-only">Loading…</span> + <span aria-hidden="true" class="size-1.5 animate-dot-pulse motion-reduce:animate-none rounded-full bg-gray-500 opacity-50"></span> + <span aria-hidden="true" class="size-1.5 animate-dot-pulse motion-reduce:animate-none [animation-delay:300ms] rounded-full bg-gray-500 opacity-50"></span> + <span aria-hidden="true" class="size-1.5 animate-dot-pulse motion-reduce:animate-none [animation-delay:600ms] rounded-full bg-gray-500 opacity-50"></span>tailwind.config.js (1)
75-84: Provide animate-delay utility or use arbitrary animation-delay
Two usages ofanimate-delay-300andanimate-delay-[600ms]remain incomponents/probe/LogLoader.vue(lines 4–5). Either register theanimate-delayplugin as shown or replace these with[animation-delay:300ms]and[animation-delay:600ms].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/probe/LogLoader.vue(1 hunks)components/probe/TabLogs.vue(1 hunks)pages/probes/[id].vue(6 hunks)tailwind.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/probe/TabLogs.vue
- pages/probes/[id].vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
tailwind.config.jscomponents/probe/LogLoader.vue
🧠 Learnings (1)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/probe/TabLogs.vue (1)
95-97: Do not use client time for pagination; use last server log timestamp (+1ms)Setting
lastFetched = Date.now()risks gaps due to clock skew and the request/response window.Apply:
- if (!error.value && enabled.value) { - lastFetched.value = Date.now(); - } + if (!error.value && enabled.value) { + const items = data.value || []; + if (items.length) { + const lastTs = new Date(items[items.length - 1]?.timestamp ?? 0).getTime(); + if (!Number.isNaN(lastTs)) lastFetched.value = lastTs + 1; // avoid >= duplicates + } + }
🧹 Nitpick comments (3)
components/probe/TabLogs.vue (3)
22-24: Avoid index as v-for key; it forces full re-render after trimsWhen you
slice(-MAX_LOGS), index-based keys change for every item, causing 5k-node re-renders. Use a stable composite key.Apply:
- <div - v-for="(log, index) in logs" - :key="index" + <div + v-for="(log, index) in logs" + :key="`${log.timestamp ?? ''}|${log.scope ?? ''}|${log.level ?? ''}|${log.message}`"If you expect duplicate messages at the same timestamp, consider tagging each item with a local
uidwhen appending.Also applies to: 138-140
70-70: Use portable timer type
NodeJS.Timeoutcan conflict with DOM typings; prefer the return type ofsetInterval.Apply:
-const refreshInterval = ref<NodeJS.Timeout>(); +const refreshInterval = ref<ReturnType<typeof setInterval>>();
146-153: Clarify pause/resume behavior: resuming drops backlogToggling “Live tail” off stops appends and, on resume, resets
lastFetchedto now, discarding logs produced while paused. If this is unintended, keeplastFetchedunchanged when disabled so you catch up on resume.Also applies to: 95-99
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/probe/TabLogs.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
components/probe/TabLogs.vue
🧠 Learnings (8)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-08-21T17:21:19.320Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-14T20:25:06.380Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-21T17:23:35.449Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:108-111
Timestamp: 2025-08-21T17:23:35.449Z
Learning: useLazyFetch in Nuxt 3 automatically handles request deduplication by key (URL or explicit key). It prevents multiple concurrent identical requests using either 'cancel' (default - abort existing and start new) or 'defer' (wait for pending request to complete) strategies. This is request-level deduplication, not data-level deduplication.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-15T10:55:44.287Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:479-486
Timestamp: 2025-08-15T10:55:44.287Z
Learning: In Nuxt's useLazyAsyncData, the loading/pending state is not initially false when the composable hasn't started yet. The loading state behavior differs from what might be expected.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 doesn't cache data by default. It uses keys primarily to avoid fetch duplication, not for cache isolation. The default auto-generated key based on file path and line number is typically sufficient unless there's a specific need for custom key generation.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 uses keys primarily for deduplication (preventing duplicate fetches), not for cache isolation. Default auto-generated keys based on file path and line number are sufficient for most use cases. There's no persistent caching by default, so explicit keys are only needed when you want to share data across different components or calls, not to prevent "cache conflicts."
Applied to files:
components/probe/TabLogs.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
…gs and simplify LogLoader condition
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
components/probe/TabLogs.vue (4)
28-31: Use a stable unique key for log rows (avoid DOM thrash on trims).Index keys will cause excessive re-renders when slicing the head of the array. Prefer a per-log ID (Redis entry ID) if available.
- v-for="(log, index) in logs" - :key="index" + v-for="(log, index) in logs" + :key="log.id || log.redisId || `${log.timestamp}-${index}`"
81-88: Fix timer typing and guarantee a single scheduled poll.Browser setTimeout typing is not NodeJS.Timeout; also clear any existing timeout before scheduling a new one.
- const refreshTimeout = ref<NodeJS.Timeout>(); + const refreshTimeout = ref<ReturnType<typeof setTimeout> | null>(null);if (enabled.value) { - refreshTimeout.value = setTimeout(() => { + if (refreshTimeout.value) clearTimeout(refreshTimeout.value); + refreshTimeout.value = setTimeout(() => { refreshLogs(); }, REFRESH_INTERVAL); }- } else { - clearTimeout(refreshTimeout.value); + } else { + if (refreshTimeout.value) clearTimeout(refreshTimeout.value); + refreshTimeout.value = null; }onUnmounted(() => { - clearTimeout(refreshTimeout.value); + if (refreshTimeout.value) clearTimeout(refreshTimeout.value); + refreshTimeout.value = null; onScrollThrottled.cancel(); });Also applies to: 113-117, 161-163, 165-167
54-57: Show the bottom loader only while a request is in flight.Currently it animates permanently whenever “Live tail” is enabled.
- <LogLoader v-if="enabled"/> + <LogLoader v-if="enabled && pending"/>
21-21: Keep header/body spacing consistent to avoid layout jumps.Render the spacer unconditionally.
- <div v-if="logs.length" class="h-4 max-lg:h-2"/> + <div class="h-4 max-lg:h-2" aria-hidden="true"/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/probe/TabLogs.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
components/probe/TabLogs.vue
🧠 Learnings (11)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-08-21T17:21:19.320Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-14T20:25:06.380Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-21T17:23:35.449Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:108-111
Timestamp: 2025-08-21T17:23:35.449Z
Learning: useLazyFetch in Nuxt 3 automatically handles request deduplication by key (URL or explicit key). It prevents multiple concurrent identical requests using either 'cancel' (default - abort existing and start new) or 'defer' (wait for pending request to complete) strategies. This is request-level deduplication, not data-level deduplication.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-15T10:55:44.287Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:479-486
Timestamp: 2025-08-15T10:55:44.287Z
Learning: In Nuxt's useLazyAsyncData, the loading/pending state is not initially false when the composable hasn't started yet. The loading state behavior differs from what might be expected.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 doesn't cache data by default. It uses keys primarily to avoid fetch duplication, not for cache isolation. The default auto-generated key based on file path and line number is typically sufficient unless there's a specific need for custom key generation.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-08-12T11:48:01.552Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/[id].vue:185-195
Timestamp: 2025-08-12T11:48:01.552Z
Learning: useAsyncData in Nuxt 3 uses keys primarily for deduplication (preventing duplicate fetches), not for cache isolation. Default auto-generated keys based on file path and line number are sufficient for most use cases. There's no persistent caching by default, so explicit keys are only needed when you want to share data across different components or calls, not to prevent "cache conflicts."
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-09-08T10:59:14.259Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:78-84
Timestamp: 2025-09-08T10:59:14.259Z
Learning: In components/probe/TabLogs.vue, the GlobalPing /v1/probes/{uuid}/logs endpoint expects a numeric epoch timestamp (milliseconds) for the `since` parameter; do not use ISO-8601.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-09-08T10:59:14.259Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:78-84
Timestamp: 2025-09-08T10:59:14.259Z
Learning: The GlobalPing API /v1/probes/{uuid}/logs endpoint does not expect ISO-8601 timestamps for the `since` parameter, contrary to typical REST API conventions.
Applied to files:
components/probe/TabLogs.vue
📚 Learning: 2025-09-08T10:58:58.498Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:146-153
Timestamp: 2025-09-08T10:58:58.498Z
Learning: The jsdelivr/globalping-dash project has SSR disabled (ssr: false in nuxt.config.ts), so warnings about server-side timer leaks or SSR-related issues are not applicable.
Applied to files:
components/probe/TabLogs.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (1)
components/probe/TabLogs.vue (1)
32-33: Guard timestamp rendering by type (avoid runtime errors).If timestamp isn’t guaranteed a string, toUpperCase() can throw. Either enforce the type in ProbeLog or guard here.
- <span v-if="log.timestamp" class="text-gray-600 dark:text-gray-500">[{{ log.timestamp.toUpperCase() }}] </span> + <span v-if="typeof log.timestamp === 'string'" class="text-gray-600 dark:text-gray-500">[{{ log.timestamp.toUpperCase() }}] </span>If ProbeLog.timestamp is always string, ignore and mark the type accordingly.
MartinKolarik
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
utils/date-formatters.ts (1)
19-29: Handle invalid dates to prevent crashes (NaN/RangeError paths).
toISOString()throws on invalid dates.getRelativeTimeStringcan computeunitIndex === -1, causing out-of-bounds access.export const formatDateForTable = (date: string | Date | null): string => { if (!date) { return ''; } if (typeof date === 'string') { date = new Date(date); } - return date.toISOString().split('T')[0]!; + if (isNaN(date.getTime())) { + return ''; + } + return date.toISOString().split('T')[0]!; }; @@ -export function getRelativeTimeString (date: Date | string, noTime: boolean = false) { +export function getRelativeTimeString (date: Date | string, noTime: boolean = false): string { if (!date) { return ''; } @@ - const timeMs = date.getTime(); + const timeMs = date.getTime(); + if (!Number.isFinite(timeMs)) { + return ''; + } @@ - const divisor = unitIndex ? cutoffs[unitIndex - 1]! : 1; + const divisor = unitIndex > 0 ? cutoffs[unitIndex - 1]! : 1; @@ - return capitalize(rtf.format(Math.ceil(deltaSeconds / divisor), units[unitIndex]!)); + const unit = units[Math.max(0, unitIndex)]!; + return capitalize(rtf.format(Math.ceil(deltaSeconds / divisor), unit));Also applies to: 67-72
pages/probes/index.vue (1)
366-379: Guard against undefinedsumin Directus aggregate transform
data[0]?.sum.amountwill throw ifsumis undefined. Use optional chaining onsumas well.- transform: data => data[0]?.sum.amount ?? 0, + transform: data => data?.[0]?.sum?.amount ?? 0,pages/probes/[id].vue (1)
80-87: Guard optional altIps to avoid runtime error.
altIps may be undefined; this v-if will crash when missing.- <button - v-if="probeDetails.altIps.length > 1" + <button + v-if="(probeDetails.altIps?.length || 0) > 1" :aria-label="showMoreIps ? 'Show fewer IPs' : 'Show more IPs'"
♻️ Duplicate comments (2)
types.ts (1)
85-86: USstate/stateNameshould remain nullable (or be normalized at fetch time)These fields were tightened to non-null for US, but:
- City.state/stateName (Lines 22-24) are still nullable for US, creating inconsistency.
- Directus data may have nulls for legacy/partial records; typing these as required risks lying to TS and masking nulls at runtime.
Either keep them nullable or guarantee normalization in a transform layer before typing as
string.- state: TCountry extends 'US' ? string : null; - stateName: TCountry extends 'US' ? string : null; + state: TCountry extends 'US' ? string | null : null; + stateName: TCountry extends 'US' ? string | null : null;If you prefer required types, add a fetch transform that fills missing US state/stateName (e.g., from geodata) and narrow types post-transform.
pages/probes/[id].vue (1)
99-112: Gate Logs by probe status and fix focus/conditional rendering.
Show Logs only when the probe is ready; also avoid focus on hidden/disabled tabs. Auth is handled upstream; keep admin check if intended.- <Tab value="details" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Details</Tab> - <Tab v-if="auth.isAdmin" value="logs" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Logs</Tab> + <Tab value="details" tabindex="0" class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto">Details</Tab> + <Tab + v-if="auth.isAdmin && isLogsEnabled" + value="logs" + :tabindex="isLogsEnabled ? 0 : -1" + class="!w-1/2 border-none !px-6 !py-2 !text-[14px] !font-bold sm:!w-auto" + > + Logs + </Tab>- <TabPanel class="flex min-h-0 flex-1 flex-col" value="logs" tabindex="-1"> - <ProbeTabLogs :probe-id="probeDetails.id"/> - </TabPanel> + <TabPanel v-if="isLogsEnabled" class="flex min-h-0 flex-1 flex-col" value="logs" tabindex="-1"> + <ProbeTabLogs :probe-id="probeDetails.id"/> + </TabPanel>Add in <script setup> (outside shown range):
const isLogsEnabled = computed(() => probeDetails.value?.status === 'ready');
🧹 Nitpick comments (14)
components/AdminPanel.vue (1)
93-95: Avoid non-null assertions; destructure to guarantee presence at compile-time.You already guard with length === 1. Destructure to drop the
!and keep TS happy.- if (impersonationList.value.length === 1) { - applyImpersonation(impersonationList.value[0]!); - } + if (impersonationList.value.length === 1) { + const [only] = impersonationList.value; + if (only) applyImpersonation(only); + }components/probe/UserTagsBlock.vue (1)
187-187: Guard against empty prefix array to prevent runtime crash.If
uPrefixesends up empty (unexpected user data),uPrefixes[0]!will throw. Provide a safe fallback to the user's default prefix.-const getDefaultEmptyTags = () => [{ uPrefix: uPrefixes[0]!, value: '' }]; +const getDefaultEmptyTags = () => [ + { uPrefix: (uPrefixes[0] ?? `u-${user.value.default_prefix}`), value: '' } +];Would you confirm the invariant “
uPrefixes.length >= 1” always holds at runtime?components/TagsList.vue (1)
36-42: Remove!by checking the indexed value once per iteration.This keeps strict indexing without assertions and avoids double indexing.
- for (let i = 0; i < props.tags.length; i++) { - if (length + props.tags[i]!.length > maxChars) { - return Math.max(1, i); - } - - length += props.tags[i]!.length; - } + for (let i = 0; i < props.tags.length; i++) { + const tag = props.tags[i]; + if (!tag) return Math.max(1, i); + if (length + tag.length > maxChars) return Math.max(1, i); + length += tag.length; + }utils/send-toast.ts (1)
10-10: Avoid noisy production console errors; gate logging to dev or lower severity.Unfiltered
console.errorcan spam logs and leak details in prod.-export const sendErrorToast = (error: unknown) => { - console.error(error); +export const sendErrorToast = (error: unknown) => { + if (process.dev) { + console.error(error); + }types.ts (1)
205-210: TightenProbeLogfor safer UI use (levels union, timestamp presence)If the API always provides a timestamp and bounded levels, reflect that to catch mistakes and simplify consumers (sorting/formatting).
type ProbeLog = { message: string; scope?: string; - level?: string; - timestamp?: string; + level?: 'debug' | 'info' | 'warn' | 'error'; + timestamp: string; // ISO 8601 from API };If
timestampcan be absent, keep it optional but consider documenting expected format.composables/useProbeFilters.ts (1)
26-32: ExportSTATUS_CODEShere to avoid ad-hoc casts elsewhere
Object.keys(STATUS_MAP) as StatusCode[]is repeated in pages and invites drift. Export a singleSTATUS_CODESfrom this module and import it where needed.export const STATUS_MAP: Record<StatusCode, StatusOption> = { 'all': { name: 'All', options: [ ...ONLINE_STATUSES, ...OFFLINE_STATUSES ] }, 'online': { name: 'Online', options: ONLINE_STATUSES }, 'online-outdated': { name: 'Online - outdated', options: ONLINE_STATUSES, outdatedOnly: true }, 'ping-test-failed': { name: 'Online - ping test failed', options: [ 'ping-test-failed' ] }, 'offline': { name: 'Offline', options: OFFLINE_STATUSES }, } as const; + +export const STATUS_CODES = Object.keys(STATUS_MAP) as StatusCode[];pages/probes/index.vue (4)
18-18: Removepiclass from NuxtIcon to avoid PrimeIcons style bleed
NuxtIcondoesn’t needpi; keeping it can accidentally pull PrimeIcons styles.- <NuxtIcon class="pi" name="capture" aria-hidden="true"/> + <NuxtIcon name="capture" aria-hidden="true"/>
55-61: Dropoption-label="code"; options are strings, not objectsPrimeVue’s
option-labelis for object options. With string options (StatusCode), it’s ignored and confusing. Clean it up.- option-label="code"Also applies to: 75-79
472-473: Avoid non-null assertion onstatusCountsaccessUse a safe fallback to prevent surprises if keys change or data is temporarily missing.
- paginatedRecords.value = newStatusCounts[filter.value.status]!; + paginatedRecords.value = newStatusCounts[filter.value.status] ?? 0;
523-526: Remove non-null assertions by structuring the dataInstead of
columns[2]!etc., destructure the known indices (tuple intent) to let TS infer non-null.// Above the return: const [ , , nameCol, locationCol, tagsCol ] = columns; // Replace the return object: return { name: nameCol.width, location: locationCol.width, tags: tagsCol.width, };components/BigIcon.vue (1)
3-3: Hide decorative icon from screen readers.
Add aria-hidden to avoid duplicate announcements when text is adjacent.- <NuxtIcon class="text-base text-primary" :class="{'text-xl': border}" :name="name" :filled="filled"/> + <NuxtIcon class="text-base text-primary" :class="{'text-xl': border}" :name="name" :filled="filled" aria-hidden="true"/>components/NuxtIcon.vue (2)
38-41: Avoid double initial fetch; watch only the prop you need.
Top-level await + watchEffect(getIcon) triggers two loads. Watch props.name with immediate instead.- await getIcon(); - - watchEffect(getIcon); + watch(() => props.name, () => { getIcon(); }, { immediate: true });Add import:
-import { ref, watchEffect } from '#imports'; +import { ref, watch, watchEffect } from '#imports';
17-19: Tighten type.
icon is always a string with ?raw; remove the union.-const icon = ref<string | Record<string, any>>(''); +const icon = ref<string>('');components/AdoptProbe.vue (1)
292-305: Rename shadowed variable to avoid confusion.
newProbes (local) shadows the outer ref newProbes; use a distinct name.- const newProbes = currentProbes.filter(probe => !initialIds.value.has(probe.id)); - if (newProbes.length) { newProbesFound(newProbes); return; } + const fresh = currentProbes.filter(probe => !initialIds.value.has(probe.id)); + if (fresh.length) { newProbesFound(fresh); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
assets/css/global.css(0 hunks)components/AdminPanel.vue(1 hunks)components/AdoptProbe.vue(6 hunks)components/BigIcon.vue(1 hunks)components/CreditsChart.vue(2 hunks)components/DeleteProbes.vue(1 hunks)components/NuxtIcon.vue(1 hunks)components/TagsList.vue(1 hunks)components/probe/UserTagsBlock.vue(1 hunks)composables/useProbeFilters.ts(1 hunks)layouts/default.vue(3 hunks)nuxt.config.ts(1 hunks)package.json(0 hunks)pages/credits.vue(1 hunks)pages/index.vue(1 hunks)pages/probes/[id].vue(7 hunks)pages/probes/index.vue(9 hunks)tsconfig.json(1 hunks)types.ts(2 hunks)utils/date-formatters.ts(3 hunks)utils/send-toast.ts(1 hunks)
💤 Files with no reviewable changes (2)
- assets/css/global.css
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- nuxt.config.ts
- layouts/default.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{vue,ts,js}
⚙️ CodeRabbit configuration file
We use Nuxt with auto imports enabled. Don't warn about missing imports.
Files:
pages/index.vuecomponents/AdminPanel.vuecomponents/DeleteProbes.vueutils/send-toast.tscomponents/CreditsChart.vuecomposables/useProbeFilters.tspages/credits.vuecomponents/TagsList.vuecomponents/BigIcon.vueutils/date-formatters.tscomponents/NuxtIcon.vuecomponents/probe/UserTagsBlock.vuepages/probes/[id].vuecomponents/AdoptProbe.vuepages/probes/index.vuetypes.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#116
File: pages/probes/index.vue:420-421
Timestamp: 2025-08-14T20:25:06.380Z
Learning: In the GlobalPing Dashboard, PavelKopecky prefers to minimize postponed fetches and accepts minimal performance impact over conditional loading optimizations. He prioritizes predictable immediate data availability over micro-optimizations that gate fetches based on user state.
📚 Learning: 2025-07-17T15:38:07.219Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:259-264
Timestamp: 2025-07-17T15:38:07.219Z
Learning: The DeleteProbes component in the GlobalPing Dashboard handles error scenarios automatically and shows error notifications using toast messages internally. No additional error handling is required in parent components when using this component.
Applied to files:
components/DeleteProbes.vue
📚 Learning: 2025-07-17T21:20:55.175Z
Learnt from: MartinKolarik
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:34-39
Timestamp: 2025-07-17T21:20:55.175Z
Learning: In the GlobalPing Dashboard codebase, prefer using `type: Array as PropType<Probe[]>` for prop definitions without importing PropType from 'vue'. This maintains runtime type checking while avoiding extra imports.
Applied to files:
components/DeleteProbes.vue
📚 Learning: 2025-07-18T09:57:23.099Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:24-26
Timestamp: 2025-07-18T09:57:23.099Z
Learning: In the GlobalPing Dashboard DeleteProbes component, the event emission pattern should ensure only one event is emitted per action - either 'success' OR 'cancel', never both. The original issue was double emission of events, not the specific events themselves.
Applied to files:
components/DeleteProbes.vue
📚 Learning: 2025-07-18T09:21:36.375Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: composables/useProbeFilters.ts:1-13
Timestamp: 2025-07-18T09:21:36.375Z
Learning: In the GlobalPing Dashboard codebase, all types are exported globally and available without explicit imports. The Status type and other types from types.ts can be used directly without import statements due to Nuxt's auto-import configuration.
Applied to files:
composables/useProbeFilters.ts
📚 Learning: 2025-08-21T17:21:19.320Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#122
File: components/probe/TabLogs.vue:52-63
Timestamp: 2025-08-21T17:21:19.320Z
Learning: In the GlobalPing Dashboard, authentication for the probe logs component (TabLogs.vue) is handled at a higher level - the component is only shown to authenticated users and admins, so individual API error handling for 401/403 responses within the component is unnecessary.
Applied to files:
pages/probes/[id].vue
📚 Learning: 2025-07-17T14:33:26.596Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: pages/probes/index.vue:416-428
Timestamp: 2025-07-17T14:33:26.596Z
Learning: In the GlobalPing Dashboard probes listing page, there's a performance optimization where an initial lightweight probe count check is performed in onMounted() using only getUserFilter('userId'). This avoids running the more expensive loadLazyData() function (which includes complex filtering and status aggregation) when a user has no probes at all. The loadLazyData() function is only called if the initial count check confirms probes exist.
Applied to files:
pages/probes/[id].vuecomponents/AdoptProbe.vuepages/probes/index.vue
📚 Learning: 2025-04-29T21:58:13.738Z
Learnt from: xbpcb
PR: jsdelivr/globalping-dash#72
File: pages/probes/[id].vue:511-519
Timestamp: 2025-04-29T21:58:13.738Z
Learning: In the GlobalPing Dashboard application, components fully unmount and remount when navigating between different IDs in dynamic routes (e.g., from `/probes/123` to `/probes/456`), so explicit watchers for route parameter changes are not required.
Applied to files:
pages/probes/[id].vue
📚 Learning: 2025-07-18T09:57:23.099Z
Learnt from: PavelKopecky
PR: jsdelivr/globalping-dash#103
File: components/DeleteProbes.vue:24-26
Timestamp: 2025-07-18T09:57:23.099Z
Learning: In the GlobalPing Dashboard DeleteProbes component, buttons should use :aria-disabled="loading" instead of :disabled="loading" during loading states. This provides accessibility information without completely preventing interaction, which is the preferred approach for this component.
Applied to files:
pages/probes/index.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run e2e tests
- GitHub Check: Run e2e tests
🔇 Additional comments (5)
tsconfig.json (1)
7-9: Confirm repo-wide readiness fornoUncheckedIndexedAccess.Attempted automated verification failed: package.json has no "typecheck" script (npm: "Missing script: "typecheck"") and the previous rg call used an invalid type flag. Run these locally and paste results:
- Typecheck:
pnpm exec tsc -p tsconfig.json --noEmit || npx -p typescript tsc -p tsconfig.json --noEmit- Search for indexed-access non-null assertions:
rg -nP '\[[^\]]+\]!' --glob '!node_modules/**' --glob '*.ts' --glob '*.vue' -C1Fix any TS errors (indexed-accesses that can be undefined) and verify UI-critical paths (components/stores/pages) are unaffected.
components/CreditsChart.vue (1)
138-149: No action needed on the tooltip guard; safe indexing is already in place.pages/probes/index.vue (1)
151-151: No action neededIcon swap looks consistent across desktop/mobile footers.
Also applies to: 229-229
pages/index.vue (1)
43-43: No change needed here.
Icon swap looks fine and already includes aria-hidden.pages/credits.vue (1)
49-53: No change needed here.
A11y is already handled via aria-hidden.
| activateCallback('5'); | ||
| } catch (e: any) { | ||
| console.error(e); | ||
| const detail = e.errors ?? 'Request failed'; | ||
| isIpValid.value = false; | ||
| invalidIpMessage.value = detail; | ||
| isResendingCode.value = true; | ||
| } |
There was a problem hiding this comment.
Surface a clean error message for invalid IP.
e.errors may be an array/object; rendering it directly yields “[object Object]”.
- const detail = e.errors ?? 'Request failed';
+ const detail =
+ (Array.isArray(e?.errors) && e.errors[0]?.message) ||
+ e?.errors?.message ||
+ e?.message ||
+ 'Request failed';
isIpValid.value = false;
invalidIpMessage.value = detail;📝 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.
| activateCallback('5'); | |
| } catch (e: any) { | |
| console.error(e); | |
| const detail = e.errors ?? 'Request failed'; | |
| isIpValid.value = false; | |
| invalidIpMessage.value = detail; | |
| isResendingCode.value = true; | |
| } | |
| activateCallback('5'); | |
| } catch (e: any) { | |
| console.error(e); | |
| const detail = | |
| (Array.isArray(e?.errors) && e.errors[0]?.message) || | |
| e?.errors?.message || | |
| e?.message || | |
| 'Request failed'; | |
| isIpValid.value = false; | |
| invalidIpMessage.value = detail; | |
| isResendingCode.value = true; | |
| } |
🤖 Prompt for AI Agents
In components/AdoptProbe.vue around lines 369 to 376, the catch handler sets
detail = e.errors which can be an array or object and renders as “[object
Object]”; replace that with a safe stringification: if e.errors is an array join
its elements with commas, if it’s an object JSON.stringify it (or extract a
useful field like message if present), otherwise cast to String, and default to
'Request failed' — assign that string to invalidIpMessage.value so the UI shows
a clean human-readable error.
| <p>You are about to delete the probe <span class="font-bold">{{ probes[0]!.name || probes[0]!.city }}</span> ({{ probes[0]!.ip }}).</p> | ||
| <p>Are you sure you want to delete this probe? You will not be able to undo this action.</p> | ||
| </div> |
There was a problem hiding this comment.
Fix potential crash when probes.length === 0.
v-else also matches the zero case, and probes[0]! will be undefined. Add an explicit single-probe branch and a safe fallback.
- <div v-else class="flex flex-col">
- <p>You are about to delete the probe <span class="font-bold">{{ probes[0]!.name || probes[0]!.city }}</span> ({{ probes[0]!.ip }}).</p>
- <p>Are you sure you want to delete this probe? You will not be able to undo this action.</p>
- </div>
+ <div v-else-if="probes.length === 1" class="flex flex-col">
+ <p>
+ You are about to delete the probe
+ <span class="font-bold">{{ (probes[0]?.name || probes[0]?.city) ?? 'Unnamed' }}</span>
+ ({{ probes[0]?.ip ?? 'unknown IP' }}).
+ </p>
+ <p>Are you sure you want to delete this probe? You will not be able to undo this action.</p>
+ </div>
+ <div v-else class="flex flex-col">
+ <p>No probes selected.</p>
+ </div>📝 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.
| <p>You are about to delete the probe <span class="font-bold">{{ probes[0]!.name || probes[0]!.city }}</span> ({{ probes[0]!.ip }}).</p> | |
| <p>Are you sure you want to delete this probe? You will not be able to undo this action.</p> | |
| </div> | |
| <div v-else-if="probes.length === 1" class="flex flex-col"> | |
| <p> | |
| You are about to delete the probe | |
| <span class="font-bold">{{ (probes[0]?.name || probes[0]?.city) ?? 'Unnamed' }}</span> | |
| ({{ probes[0]?.ip ?? 'unknown IP' }}). | |
| </p> | |
| <p>Are you sure you want to delete this probe? You will not be able to undo this action.</p> | |
| </div> | |
| <div v-else class="flex flex-col"> | |
| <p>No probes selected.</p> | |
| </div> |
🤖 Prompt for AI Agents
In components/DeleteProbes.vue around lines 17 to 19, the template assumes
probes[0] exists which will crash when probes.length === 0 because the v-else
branch also matches the empty case; add an explicit branch for the single-probe
case (e.g. v-if probes.length === 1) and update the other branches so the empty
array case is handled safely with a fallback message, and replace direct
probes[0]! property access with guarded access (use a conditional or optional
chaining) so rendering never dereferences undefined.
| const iconsImport = import.meta.glob('assets/icons/**/**.svg', { | ||
| eager: false, | ||
| query: '?raw', | ||
| import: 'default', | ||
| }); | ||
| const iconPath = `/assets/icons/${props.name}.svg`; | ||
| const rawIcon = await iconsImport[iconPath]!() as string; | ||
| if (rawIcon.includes('stroke')) { hasStroke = true; } | ||
| icon.value = rawIcon; | ||
| } catch { |
There was a problem hiding this comment.
Fix icon path mismatch and stale hasStroke.
- The glob keys (no leading slash) won’t match iconPath (with leading slash) → iconsImport[iconPath] is undefined.
- hasStroke is never reset; once true, it stays true across icons.
- const iconsImport = import.meta.glob('assets/icons/**/**.svg', {
+ const iconsImport = import.meta.glob('/assets/icons/**/**.svg', {
eager: false,
query: '?raw',
import: 'default',
});
const iconPath = `/assets/icons/${props.name}.svg`;
- const rawIcon = await iconsImport[iconPath]!() as string;
+ const loader = iconsImport[iconPath];
+ if (!loader) { throw new Error('not found'); }
+ const rawIcon = await loader() as string;
-
- if (rawIcon.includes('stroke')) { hasStroke = true; }
+ hasStroke = rawIcon.includes('stroke');
icon.value = rawIcon;📝 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.
| const iconsImport = import.meta.glob('assets/icons/**/**.svg', { | |
| eager: false, | |
| query: '?raw', | |
| import: 'default', | |
| }); | |
| const iconPath = `/assets/icons/${props.name}.svg`; | |
| const rawIcon = await iconsImport[iconPath]!() as string; | |
| if (rawIcon.includes('stroke')) { hasStroke = true; } | |
| icon.value = rawIcon; | |
| } catch { | |
| const iconsImport = import.meta.glob('/assets/icons/**/**.svg', { | |
| eager: false, | |
| query: '?raw', | |
| import: 'default', | |
| }); | |
| const iconPath = `/assets/icons/${props.name}.svg`; | |
| const loader = iconsImport[iconPath]; | |
| if (!loader) { throw new Error('not found'); } | |
| const rawIcon = await loader() as string; | |
| hasStroke = rawIcon.includes('stroke'); | |
| icon.value = rawIcon; | |
| } catch { |
🤖 Prompt for AI Agents
In components/NuxtIcon.vue around lines 22 to 33, the glob lookup fails because
iconsImport keys have no leading slash and hasStroke is never reset; change
iconPath to match the glob keys (remove the leading slash or normalize both with
e.g. iconPath = `assets/icons/${props.name}.svg`) and reset or recompute
hasStroke for each load (set hasStroke = false before checking or derive it from
the newly loaded rawIcon). Also guard the lookup (check iconsImport[iconPath]
exists) and handle the missing case instead of assuming the function is present.
Closes #26
Merge after jsdelivr/globalping#710 and jsdelivr/globalping-probe#314
BREAKING CHANGE: GP_API_URL env variable now contains the API address, auth url is defined via the GP_AUTH_URL env variable