Repository navigation
fix: lambda script - #61
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Pexels image scraper Lambda function for better compatibility with AWS Lambda environment and improved debugging capabilities. The changes focus on transitioning from ARM64 to AMD64 architecture, adding extensive logging, and improving resource cleanup.
- Enhanced Playwright browser configuration with Lambda-specific arguments and improved error handling
- Added comprehensive logging throughout the scraping and Lambda handler execution
- Improved Docker configuration for AWS Lambda deployment with AMD64 architecture
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/src/main.ts | Added extensive logging, Lambda-specific Chromium configuration, improved resource cleanup with separate page/context/browser closing |
| scripts/src/lambda-handler.ts | Enhanced error logging with stack traces and added execution tracking logs |
| scripts/build-lambda.sh | Changed platform from linux/arm64 to linux/amd64, added ECR push commands with hardcoded AWS account ID |
| scripts/Dockerfile.script | Updated PLAYWRIGHT_BROWSERS_PATH, added cleanup step, and ensured chromium deps are installed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let page = null; | ||
|
|
||
| // Set environment variable to use system Chromium in Lambda | ||
| process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH = process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH || '/ms-playwright/chromium-*/chrome-linux/chrome'; |
There was a problem hiding this comment.
The wildcard path '/ms-playwright/chromium-*/chrome-linux/chrome' will not work at runtime. Shell glob patterns are not resolved when setting environment variables. You need to use an explicit path or resolve the glob pattern programmatically using the fs module.
| docker buildx build --platform linux/amd64 --provenance=false -f Dockerfile.script -t pwp/pexels-image-scraper-lambda . | ||
| # docker run --platform linux/amd64 -v ~/.aws-lambda-rie:/aws-lambda -p 9000:8080 \ | ||
| # --entrypoint /aws-lambda/aws-lambda-rie \ | ||
| # lambda-script:test \ | ||
| # /usr/local/bin/npx aws-lambda-ric dist/lambda-handler.handler | ||
|
|
||
| docker tag pwp/pexels-image-scraper-lambda:latest 279892746640.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest | ||
|
|
||
| docker push 279892746640.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest No newline at end of file |
There was a problem hiding this comment.
The AWS account ID (279892746640) is hardcoded in the script. This should be parameterized using an environment variable (e.g., AWS_ACCOUNT_ID) to avoid exposing sensitive account information and to make the script reusable across different environments.
| docker buildx build --platform linux/amd64 --provenance=false -f Dockerfile.script -t pwp/pexels-image-scraper-lambda . | |
| # docker run --platform linux/amd64 -v ~/.aws-lambda-rie:/aws-lambda -p 9000:8080 \ | |
| # --entrypoint /aws-lambda/aws-lambda-rie \ | |
| # lambda-script:test \ | |
| # /usr/local/bin/npx aws-lambda-ric dist/lambda-handler.handler | |
| docker tag pwp/pexels-image-scraper-lambda:latest 279892746640.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest | |
| docker push 279892746640.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest | |
| if [ -z "$AWS_ACCOUNT_ID" ]; then | |
| echo "Error: AWS_ACCOUNT_ID environment variable is not set." | |
| exit 1 | |
| fi | |
| docker buildx build --platform linux/amd64 --provenance=false -f Dockerfile.script -t pwp/pexels-image-scraper-lambda . | |
| # docker run --platform linux/amd64 -v ~/.aws-lambda-rie:/aws-lambda -p 9000:8080 \ | |
| # --entrypoint /aws-lambda/aws-lambda-rie \ | |
| # lambda-script:test \ | |
| # /usr/local/bin/npx aws-lambda-ric dist/lambda-handler.handler | |
| docker tag pwp/pexels-image-scraper-lambda:latest ${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest | |
| docker push ${AWS_ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/pwp/pexels-image-scraper-lambda:latest |
| try { | ||
| page = await context.newPage(); | ||
| console.log("New page created successfully"); | ||
| } catch (pageError) { | ||
| console.error("Failed to create new page"); | ||
| console.error(`Browser connected: ${browser.isConnected()}`); | ||
| throw pageError; | ||
| } |
There was a problem hiding this comment.
This nested try-catch block adds unnecessary complexity. The pageError will be caught by the outer catch block (line 179), making this inner try-catch redundant. The browser.isConnected() check could be added to the outer error handling instead.
No description provided.