feat(frontend): adding project screenshot #159
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes update the backend and frontend code to improve file URL handling and introduce new features. The backend now uses Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as Screenshot API Route
participant B as Puppeteer Browser
participant P as Web Page
C->>A: GET /api/screenshot?url={target URL}
A->>B: Launch headless browser
B->>P: Open new page & set viewport (1280x720)
P->>P: Navigate to URL (wait for network idle)
P->>B: Capture screenshot (PNG)
B->>A: Return screenshot buffer
A->>C: Respond with image/png and cache-control header
sequenceDiagram
participant U as User
participant E as ExpandableCard Component
participant PC as ProjectContext
U->>E: Clicks on a project card
E->>E: Check for cached URL
alt URL is cached
E->>E: Use cached URL to update iframe and activate project
else URL not cached
E->>PC: Call getWebUrl(project path)
PC-->>E: Return { domain, containerId }
E->>PC: Invoke takeProjectScreenshot(projectId, URL)
E->>E: Cache URL and update active project state
end
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
frontend/src/components/root/expand-card.tsx (1)
135-141: 🛠️ Refactor suggestionSwitch from img to Next.js Image component.
Using a standard
imgtag instead of Next.jsImagecomponent loses important optimizations like lazy loading, automatic responsive sizing, and image optimization.Restore the Next.js Image component:
-<img - src={project.image} - alt={project.name} - width={600} - height={200} - className="w-full h-48 object-cover transition duration-300 group-hover:scale-105" -/> +<Image + src={project.image} + alt={project.name} + width={600} + height={200} + className="w-full h-48 object-cover transition duration-300 group-hover:scale-105" +/>frontend/src/components/chat/code-engine/project-context.tsx (1)
423-461: 🛠️ Refactor suggestionAvoid repeating the runProject + screenshot pattern in pollChatProject.
Currently, the code runs/api/runProjectand callstakeProjectScreenshotsimilarly togetWebUrl. Consider refactoring to reusegetWebUrlinsidepollChatProjectfor consistency, unless there is a specific need to repeat this logic.
🧹 Nitpick comments (7)
frontend/src/app/api/runProject/route.ts (1)
301-301: Unnecessary empty line.This empty line doesn't affect functionality but is an unnecessary change.
Consider removing it to keep the code concise.
frontend/src/app/api/screenshot/route.ts (1)
49-55: Error handling could be enhanced.While the current error handling is functional, it could be improved to provide more specific error messages based on the type of failure.
Consider enhancing the error handling:
} catch (error: any) { - console.error('Screenshot error:', error); + console.error('Screenshot capture failed:', error); + + // Provide more specific error messages based on error type + const errorMessage = error.message || 'Failed to capture screenshot'; + const statusCode = error.name === 'TimeoutError' ? 504 : 500; + return NextResponse.json( - { error: error.message || 'Failed to capture screenshot' }, - { status: 500 } + { error: errorMessage }, + { status: statusCode } ); }frontend/src/components/chat/code-engine/web-view.tsx (1)
33-62: Consider adding user feedback during errors.The error is logged to the console, but there's no feedback to the user when URL fetching fails.
Consider updating the UI to show error states:
try { const { domain } = await getWebUrl(projectPath); containerRef.current = { projectPath, domain, }; const baseUrl = `http://${domain}`; console.log('baseUrl:', baseUrl); setBaseUrl(baseUrl); setDisplayPath('/'); } catch (error) { console.error('Error getting web URL:', error); + // Show error to user + setBaseUrl(''); + // You could also set an error state here to display in the UI + // setError('Failed to load project preview. Please try again later.'); }frontend/src/components/root/expand-card.tsx (1)
38-45: Add error handling feedback to the user.When URL fetching fails, the user should be notified of the error instead of just logging to the console.
Add user feedback for errors:
try { const data = await getWebUrl(project.path); const url = `http://${data.domain}`; cachedUrls.current.set(project.id, url); setIframeUrl(url); } catch (error) { console.error('Error fetching project URL:', error); + // Provide user feedback + setIframeUrl(''); + // You could also set an error state to display in the UI + // setError(`Failed to load project preview: ${error.message}`); + // Or show a toast notification + // toast.error('Failed to load project preview. Please try again later.'); }frontend/src/components/chat/code-engine/project-context.tsx (3)
44-48: Added methods in interface look aligned with feature requirements.
DeclaringgetWebUrlandtakeProjectScreenshotin theProjectContextTypeinterface provides clarity on the new functionalities. Consider adding inline documentation to explain these methods for easier onboarding.
227-258: Consider providing user feedback on screenshot failure.
WhiletakeProjectScreenshotusescheckUrlStatus, if the screenshot or status check fails, the user only sees a console error. For better UX, add a toast or relevant UI notification.} catch (error) { console.error('Error:', error); + toast.error('Failed to capture screenshot. Please retry or check your network.'); }
260-294: DRY up repeated logic for runProject calls.
getWebUrlduplicates similar runProject invocation and screenshot capturing logic also present inpollChatProject. Centralizing this logic could simplify maintenance and prevent divergence.
📜 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 (10)
backend/src/upload/upload.service.ts(2 hunks)frontend/package.json(1 hunks)frontend/src/app/api/runProject/route.ts(2 hunks)frontend/src/app/api/screenshot/route.ts(1 hunks)frontend/src/components/chat/code-engine/project-context.tsx(7 hunks)frontend/src/components/chat/code-engine/web-view.tsx(3 hunks)frontend/src/components/root/expand-card.tsx(3 hunks)frontend/src/graphql/request.ts(1 hunks)frontend/src/graphql/schema.gql(7 hunks)frontend/src/graphql/type.tsx(18 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: autofix
- GitHub Check: Install and Build Backend
- GitHub Check: Install and Build Frontend
🔇 Additional comments (35)
frontend/package.json (1)
54-54: Puppeteer dependency added for screenshot functionality.This addition supports the project screenshot feature mentioned in the PR title. Puppeteer is a Node.js library that provides a high-level API to control headless browsers, which is perfect for taking screenshots of web pages.
Note that Puppeteer is a relatively heavy dependency as it downloads and manages a browser instance. This will increase your package size, but it's an appropriate choice for the screenshot functionality.
frontend/src/graphql/request.ts (1)
228-236: GraphQL mutation for updating project photo URLs looks good.The mutation follows the established pattern in the codebase and properly exposes the fields needed for the screenshot feature. This aligns well with the PR objective of adding project screenshots.
frontend/src/app/api/screenshot/route.ts (6)
1-3: Import structure looks good.The imports are clean and appropriate for the functionality provided by this file.
5-14: Parameter validation is implemented correctly.The function properly checks for the required URL parameter and returns an appropriate error response when it's missing.
16-20: Headless browser launch configuration is appropriate.The configuration for launching Puppeteer is correctly set to headless mode, which is essential for server-side screenshot generation.
22-26: Viewport configuration is well-defined.Setting a reasonable viewport size ensures consistent screenshot dimensions and quality.
28-31: Navigation settings are properly configured.The
waitUntil: 'networkidle0'setting ensures that screenshots are taken only after the page is fully loaded, and the timeout value is appropriate.
43-48: Response and caching configuration is well implemented.The screenshot is correctly returned with the appropriate content type and caching headers.
frontend/src/components/chat/code-engine/web-view.tsx (4)
17-20: Good defensive programming with context validation.Validating that both
curProjectandgetWebUrlare available from the context before proceeding prevents runtime errors.
32-65: Improved URL fetching logic.The refactored
initWebUrlfunction is more efficient by directly using the context'sgetWebUrlfunction instead of making a separate API call.
60-60: Better error message wording.The updated error message is more specific and helpful for debugging.
64-65: Updated dependencies are appropriate.Adding
getWebUrlto the dependencies array ensures the effect runs when the URL retrieval method changes.frontend/src/components/root/expand-card.tsx (3)
3-6: Imports are organized appropriately.The imports are well-structured, and the addition of
useContextis appropriate for using the ProjectContext.
12-13: Context usage is implemented correctly.The destructuring of
getWebUrlandtakeProjectScreenshotfrom the context, along with the URL caching mechanism, is a good approach.
30-46: The handleCardClick function has good caching logic.Caching URLs by project ID is an excellent performance optimization to avoid redundant API calls.
frontend/src/graphql/schema.gql (8)
60-63: EmailConfirmationResponse type looks good.The new type with required message and optional success fields is appropriately designed for email confirmation responses.
109-109: Updated confirmEmail mutation signature is correct.The mutation now returns the EmailConfirmationResponse type, which provides better information to the client.
119-119: New resendConfirmationEmail mutation is appropriately defined.The mutation accepts an input type and returns an EmailConfirmationResponse, maintaining consistency with the confirmEmail mutation.
179-179: New getRemainingProjectLimit query is appropriate.Returning an integer for project limits is the correct approach.
199-201: ResendEmailInput type is correctly defined.The input type with a required email field matches the needs of the resendConfirmationEmail mutation.
238-239: User type extensions are appropriate.The addition of isEmailConfirmed and lastEmailSendTime fields to the User type provides valuable information for email confirmation status.
241-241: Good use of deprecation annotation.Using the @deprecated directive with a reason is the correct approach to mark fields that should no longer be used.
1-3:✅ Verification successful
Verify that this auto-generated schema is properly maintained.
The comment at the top indicates this file is auto-generated. Manual edits to this file might be overwritten by the schema generator.
🏁 Script executed:
#!/bin/bash # Check if there are any scripts or configuration that generate this file echo "Checking for schema generation scripts..." rg -l "schema.gql" --type=ts --type=js -g "!node_modules" -g "!dist" | grep -i "generat\|build\|schema" # Look for any indications that this file should not be edited manually echo "Checking if the file has markers indicating it shouldn't be manually edited..." cat frontend/src/graphql/schema.gql | head -5Length of output: 597
Auto-generated schema maintenance verified
The schema file (
frontend/src/graphql/schema.gql) is correctly marked as auto-generated, as confirmed by the header. There’s no indication that manual changes are expected; any necessary modifications should be made via the schema generator. No further action is required.frontend/src/components/chat/code-engine/project-context.tsx (2)
17-17: All good here.
The addition ofUPDATE_PROJECT_PHOTO_URLto the import list looks consistent with the newly introduced functionality below.
196-225: Mutation for updating project photo URL is well-structured.
TheupdateProjectPhotoMutationlogic and local state updates look correct and handle potential concurrency via the functionalsetProjects. The error handling withtoast.erroris standard.frontend/src/graphql/type.tsx (10)
99-103: New type definition for email confirmation looks good.
The introduction ofEmailConfirmationResponseis coherent. Markingsuccessas optional allows flexibility in returning partial responses.
153-153: New confirmEmail mutation recognized.
Definition is consistent with the newly added type. Ensure relevant resolvers and client calls are implemented.
163-163: resendConfirmationEmail field properly introduced.
Pairs well withconfirmEmailand theEmailConfirmationResponse. Good addition to handle multi-step email confirmations.
175-177: Arguments for confirmEmail are straightforward.
No issues found. Thetokenargument is well-named.
215-217: Arguments for resendConfirmationEmail ensure clarity.
Theinputstruct is consistent with typical GraphQL patterns.
339-341: New ResendEmailInput type is minimal and clear.
Appropriately typed withemail: string. Looks aligned with the usage.
374-375: Fields for user email confirmation status are well-defined.
isEmailConfirmedandlastEmailSendTimecomplement the new email confirmation flow.
647-655: EmailConfirmationResponse resolvers integrated correctly.
Resolvers match the schema and handle optional success property.
985-991: User resolvers for email confirmation fields are consistent with schema.
Implementation forisEmailConfirmedandlastEmailSendTimeensures consistent access to new fields in resolvers.
1012-1012: Addition of EmailConfirmationResponse to resolvers is coherent.
Keeps the generated types aligned with the new schema.
| @@ -3,6 +3,10 @@ import { exec } from 'child_process'; | |||
| import * as path from 'path'; | |||
| import * as net from 'net'; | |||
| import { getProjectPath } from 'codefox-common'; | |||
| import puppetter from 'puppeteer'; | |||
There was a problem hiding this comment.
Fix typo in puppeteer import.
There's a typo in the puppeteer import that could cause runtime errors.
-import puppetter from 'puppeteer';
+import puppeteer from 'puppeteer';📝 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.
| import puppetter from 'puppeteer'; | |
| import puppeteer from 'puppeteer'; |
| import { useMutation } from '@apollo/client/react/hooks/useMutation'; | ||
| import { toast } from 'sonner'; | ||
| import { UPDATE_PROJECT_PHOTO_URL } from '@/graphql/request'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports.
These imports are not used in this file and should be removed. In particular, useMutation is a client-side React hook that shouldn't be imported in a server-side API route.
-import { useMutation } from '@apollo/client/react/hooks/useMutation';
-import { toast } from 'sonner';
-import { UPDATE_PROJECT_PHOTO_URL } from '@/graphql/request';📝 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.
| import { useMutation } from '@apollo/client/react/hooks/useMutation'; | |
| import { toast } from 'sonner'; | |
| import { UPDATE_PROJECT_PHOTO_URL } from '@/graphql/request'; |
backend/src/upload/upload.service.ts
Outdated
| @@ -99,7 +99,7 @@ export class UploadService { | |||
| // Get the appropriate URL for the uploaded file | |||
| const bucketUrl = this.getBucketUrl(); | |||
|
|
|||
| return { url: `${bucketUrl}/${key}`, key }; | |||
| return { url: path.join(bucketUrl, key), key }; | |||
There was a problem hiding this comment.
URL construction should not use path.join.
Using path.join for URLs is problematic because it uses OS-specific path separators (backslashes on Windows), while URLs always use forward slashes regardless of the OS.
- return { url: path.join(bucketUrl, key), key };
+ return { url: `${bucketUrl}/${key}`, key };For proper URL construction, use one of these alternatives:
- String template literals:
`${bucketUrl}/${key}` - URL constructor:
new URL(key, bucketUrl).toString() - URL-specific join utility:
URLJoin(bucketUrl, key)from a package like 'url-join'
This change is needed in both places where path.join is used for URL construction.
Also applies to: 138-138
| // Take screenshot | ||
| const screenshot = await page.screenshot({ | ||
| path: `dsadas.png`, | ||
| type: 'png', | ||
| fullPage: true, | ||
| }); |
There was a problem hiding this comment.
Remove hardcoded filename in screenshot configuration.
The path: 'dsadas.png' appears to be a development/testing artifact. This will save files to disk unnecessarily in production.
Apply this diff to fix the issue:
const screenshot = await page.screenshot({
- path: `dsadas.png`,
type: 'png',
fullPage: true,
});📝 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.
| // Take screenshot | |
| const screenshot = await page.screenshot({ | |
| path: `dsadas.png`, | |
| type: 'png', | |
| fullPage: true, | |
| }); | |
| // Take screenshot | |
| const screenshot = await page.screenshot({ | |
| type: 'png', | |
| fullPage: true, | |
| }); |
| import { randomUUID } from 'crypto'; | ||
| import { NextResponse } from 'next/server'; | ||
| import puppeteer from 'puppeteer'; | ||
|
|
||
| export async function GET(req: Request) { | ||
| const { searchParams } = new URL(req.url); | ||
| const url = searchParams.get('url'); | ||
|
|
||
| if (!url) { | ||
| return NextResponse.json( | ||
| { error: 'URL parameter is required' }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const browser = await puppeteer.launch({ | ||
| headless: true, | ||
| }); | ||
| const page = await browser.newPage(); | ||
|
|
||
| // Set viewport to a reasonable size | ||
| await page.setViewport({ | ||
| width: 1280, | ||
| height: 720, | ||
| }); | ||
|
|
||
| await page.goto(url, { | ||
| waitUntil: 'networkidle0', | ||
| timeout: 30000, | ||
| }); | ||
|
|
||
| // Take screenshot | ||
| const screenshot = await page.screenshot({ | ||
| path: `dsadas.png`, | ||
| type: 'png', | ||
| fullPage: true, | ||
| }); | ||
|
|
||
| await browser.close(); | ||
|
|
||
| // Return the screenshot as a PNG image | ||
| return new Response(screenshot, { | ||
| headers: { | ||
| 'Content-Type': 'image/png', | ||
| 'Cache-Control': 's-maxage=3600', | ||
| }, | ||
| }); | ||
| } catch (error: any) { | ||
| console.error('Screenshot error:', error); | ||
| return NextResponse.json( | ||
| { error: error.message || 'Failed to capture screenshot' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add URL validation to prevent potential security issues.
This endpoint accepts any URL without validation, which could lead to security issues like server-side request forgery (SSRF).
Add URL validation before processing:
import { randomUUID } from 'crypto';
import { NextResponse } from 'next/server';
import puppeteer from 'puppeteer';
+// Helper function to validate URL
+function isValidUrl(urlString: string): boolean {
+ try {
+ const url = new URL(urlString);
+ // Only allow http and https protocols
+ return url.protocol === 'http:' || url.protocol === 'https:';
+ } catch {
+ return false;
+ }
+}
export async function GET(req: Request) {
const { searchParams } = new URL(req.url);
const url = searchParams.get('url');
if (!url) {
return NextResponse.json(
{ error: 'URL parameter is required' },
{ status: 400 }
);
}
+ if (!isValidUrl(url)) {
+ return NextResponse.json(
+ { error: 'Invalid URL provided' },
+ { status: 400 }
+ );
+ }
try {
// ... rest of the function remains the same📝 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.
| import { randomUUID } from 'crypto'; | |
| import { NextResponse } from 'next/server'; | |
| import puppeteer from 'puppeteer'; | |
| export async function GET(req: Request) { | |
| const { searchParams } = new URL(req.url); | |
| const url = searchParams.get('url'); | |
| if (!url) { | |
| return NextResponse.json( | |
| { error: 'URL parameter is required' }, | |
| { status: 400 } | |
| ); | |
| } | |
| try { | |
| const browser = await puppeteer.launch({ | |
| headless: true, | |
| }); | |
| const page = await browser.newPage(); | |
| // Set viewport to a reasonable size | |
| await page.setViewport({ | |
| width: 1280, | |
| height: 720, | |
| }); | |
| await page.goto(url, { | |
| waitUntil: 'networkidle0', | |
| timeout: 30000, | |
| }); | |
| // Take screenshot | |
| const screenshot = await page.screenshot({ | |
| path: `dsadas.png`, | |
| type: 'png', | |
| fullPage: true, | |
| }); | |
| await browser.close(); | |
| // Return the screenshot as a PNG image | |
| return new Response(screenshot, { | |
| headers: { | |
| 'Content-Type': 'image/png', | |
| 'Cache-Control': 's-maxage=3600', | |
| }, | |
| }); | |
| } catch (error: any) { | |
| console.error('Screenshot error:', error); | |
| return NextResponse.json( | |
| { error: error.message || 'Failed to capture screenshot' }, | |
| { status: 500 } | |
| ); | |
| } | |
| } | |
| import { randomUUID } from 'crypto'; | |
| import { NextResponse } from 'next/server'; | |
| import puppeteer from 'puppeteer'; | |
| // Helper function to validate URL | |
| function isValidUrl(urlString: string): boolean { | |
| try { | |
| const url = new URL(urlString); | |
| // Only allow http and https protocols | |
| return url.protocol === 'http:' || url.protocol === 'https:'; | |
| } catch { | |
| return false; | |
| } | |
| } | |
| export async function GET(req: Request) { | |
| const { searchParams } = new URL(req.url); | |
| const url = searchParams.get('url'); | |
| if (!url) { | |
| return NextResponse.json( | |
| { error: 'URL parameter is required' }, | |
| { status: 400 } | |
| ); | |
| } | |
| if (!isValidUrl(url)) { | |
| return NextResponse.json( | |
| { error: 'Invalid URL provided' }, | |
| { status: 400 } | |
| ); | |
| } | |
| try { | |
| const browser = await puppeteer.launch({ | |
| headless: true, | |
| }); | |
| const page = await browser.newPage(); | |
| // Set viewport to a reasonable size | |
| await page.setViewport({ | |
| width: 1280, | |
| height: 720, | |
| }); | |
| await page.goto(url, { | |
| waitUntil: 'networkidle0', | |
| timeout: 30000, | |
| }); | |
| // Take screenshot | |
| const screenshot = await page.screenshot({ | |
| path: `dsadas.png`, | |
| type: 'png', | |
| fullPage: true, | |
| }); | |
| await browser.close(); | |
| // Return the screenshot as a PNG image | |
| return new Response(screenshot, { | |
| headers: { | |
| 'Content-Type': 'image/png', | |
| 'Cache-Control': 's-maxage=3600', | |
| }, | |
| }); | |
| } catch (error: any) { | |
| console.error('Screenshot error:', error); | |
| return NextResponse.json( | |
| { error: error.message || 'Failed to capture screenshot' }, | |
| { status: 500 } | |
| ); | |
| } | |
| } |
| try { | ||
| const browser = await puppeteer.launch({ | ||
| headless: true, | ||
| }); | ||
| const page = await browser.newPage(); | ||
|
|
||
| // Set viewport to a reasonable size | ||
| await page.setViewport({ | ||
| width: 1280, | ||
| height: 720, | ||
| }); | ||
|
|
||
| await page.goto(url, { | ||
| waitUntil: 'networkidle0', | ||
| timeout: 30000, | ||
| }); | ||
|
|
||
| // Take screenshot | ||
| const screenshot = await page.screenshot({ | ||
| path: `dsadas.png`, | ||
| type: 'png', | ||
| fullPage: true, | ||
| }); | ||
|
|
||
| await browser.close(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider resource cleanup in case of errors.
The browser should be closed even if an error occurs during screenshot capture to prevent memory leaks.
Refactor to ensure the browser is always closed:
try {
const browser = await puppeteer.launch({
headless: true,
});
const page = await browser.newPage();
+ try {
// Set viewport to a reasonable size
await page.setViewport({
width: 1280,
height: 720,
});
await page.goto(url, {
waitUntil: 'networkidle0',
timeout: 30000,
});
// Take screenshot
const screenshot = await page.screenshot({
type: 'png',
fullPage: true,
});
await browser.close();
// Return the screenshot as a PNG image
return new Response(screenshot, {
headers: {
'Content-Type': 'image/png',
'Cache-Control': 's-maxage=3600',
},
});
+ } catch (error) {
+ await browser.close();
+ throw error;
+ }
} catch (error: any) {
console.error('Screenshot error:', error);
return NextResponse.json(
{ error: error.message || 'Failed to capture screenshot' },
{ status: 500 }
);
}| @@ -11,9 +11,13 @@ import { | |||
| ZoomIn, | |||
| ZoomOut, | |||
| } from 'lucide-react'; | |||
| import puppeteer from 'puppeteer'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused Puppeteer import.
The puppeteer import is not used in this component and should be removed.
-import puppeteer from 'puppeteer';📝 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.
| import puppeteer from 'puppeteer'; |
| onClick={async () => { | ||
| const data = await getWebUrl(project.path); | ||
|
|
||
| console.log(project.image); | ||
| const url = `http://${data.domain}`; | ||
| setIframeUrl(url); | ||
| handleCardClick(project); | ||
| setActive(project); | ||
| }} |
There was a problem hiding this comment.
Fix redundant API calls in click handler.
The click handler makes the same API call twice: once directly and once through handleCardClick. This causes unnecessary network requests and potential race conditions.
Apply this diff to fix the redundant API calls:
onClick={async () => {
- const data = await getWebUrl(project.path);
-
- console.log(project.image);
- const url = `http://${data.domain}`;
- setIframeUrl(url);
handleCardClick(project);
- setActive(project);
}}| const checkUrlStatus = async (url: string) => { | ||
| let status = 0; | ||
| while (status !== 200) { | ||
| try { | ||
| const res = await fetch(url, { method: 'HEAD' }); | ||
| status = res.status; | ||
| if (status !== 200) { | ||
| console.log(`URL status: ${status}. Retrying...`); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| } | ||
| } catch (err) { | ||
| console.error('Error checking URL status:', err); | ||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Prevent potential infinite loop by adding a max retry limit.
The current checkUrlStatus function loops indefinitely until a 200 response is returned. This can lead to a hang if the URL never becomes valid. Incorporate a maximum retry or timeout to avoid blocking.
- while (status !== 200) {
+ let attempts = 0;
+ const maxAttempts = 20;
+ while (status !== 200 && attempts < maxAttempts) {
try {
// ...
} catch (err) {
// ...
}
+ attempts++;
}📝 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 checkUrlStatus = async (url: string) => { | |
| let status = 0; | |
| while (status !== 200) { | |
| try { | |
| const res = await fetch(url, { method: 'HEAD' }); | |
| status = res.status; | |
| if (status !== 200) { | |
| console.log(`URL status: ${status}. Retrying...`); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| } catch (err) { | |
| console.error('Error checking URL status:', err); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| } | |
| }; | |
| const checkUrlStatus = async (url: string) => { | |
| let status = 0; | |
| let attempts = 0; | |
| const maxAttempts = 20; | |
| while (status !== 200 && attempts < maxAttempts) { | |
| try { | |
| const res = await fetch(url, { method: 'HEAD' }); | |
| status = res.status; | |
| if (status !== 200) { | |
| console.log(`URL status: ${status}. Retrying...`); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| } catch (err) { | |
| console.error('Error checking URL status:', err); | |
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |
| } | |
| attempts++; | |
| } | |
| }; |
Sma1lboy
changed the title
Summary by CodeRabbit
New Features
Chores