stephen/apartment-dashboard-api
1
0
Fork 0
Code Issues 1 Pull Requests Actions Releases Wiki Activity

SCRAPE-3: Implement fetchPage() with axios, timeout, retry #8

Merged
stephen merged 1 commits from scraper/fetch-page into dev 2026-02-05 15:58:08 -07:00
Conversation 2 Commits 1 Files Changed 3 +651 -2
stephen commented 2026-01-30 11:25:03 -07:00
Owner
Copy Link

Summary

  • Add fetchPage() function using axios for HTTP GET requests
  • Implement configurable timeout (default 30s) and User-Agent header
  • Add retry logic with exponential backoff (1s, 2s, 4s) for 5xx errors and network timeouts
  • Skip retry on 4xx client errors
  • Add comprehensive logging for each attempt with error details

Test Plan

  • All 26 fetchPage tests pass
  • All 61 scraper tests pass
  • Manual verification done
## Summary - Add fetchPage() function using axios for HTTP GET requests - Implement configurable timeout (default 30s) and User-Agent header - Add retry logic with exponential backoff (1s, 2s, 4s) for 5xx errors and network timeouts - Skip retry on 4xx client errors - Add comprehensive logging for each attempt with error details ## Test Plan - [x] All 26 fetchPage tests pass - [x] All 61 scraper tests pass - [ ] Manual verification done
stephen added 4 commits 2026-01-30 11:25:03 -07:00
Add ESLint and n8n webhook notifications to CI/CD 6daacf4d12 
- Add ESLint 9 with flat config for Node.js linting
- Add lint and lint:fix npm scripts
- Add lint job to CI/CD pipeline
- Add notify job to send test/lint results to n8n webhook
- Webhook reports pass/fail status with failure details

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add scraperLogger.js with credential redaction (#6) 9dba679221 
Co-authored-by: Stephen Minakian <stephenminakian@gmail.com>
Co-committed-by: Stephen Minakian <stephenminakian@gmail.com>
Add PR number to webhook payload 3af5a90c93 
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add fetchPage() with axios, timeout, and retry logic
All checks were successful
CI/CD Pipeline - Apartment API / Scan Dependencies (pull_request) Successful in 12s
Details
CI/CD Pipeline - Apartment API / Run Linting (pull_request) Successful in 9m36s
Details
CI/CD Pipeline - Apartment API / Run Tests (pull_request) Successful in 9m42s
Details
CI/CD Pipeline - Apartment API / Send Webhook Notification (pull_request) Successful in 2s
Details
CI/CD Pipeline - Apartment API / Build & Push Image (pull_request) Has been skipped
Details
CI/CD Pipeline - Apartment API / Deploy to Production (pull_request) Has been skipped
Details
19baf43493 
Implement fetchPage() function for HTTP scraping:
- Uses axios for HTTP GET requests
- Configurable timeout (default 30s) and User-Agent header
- Retry with exponential backoff (1s, 2s, 4s) on 5xx and network errors
- Does not retry on 4xx client errors
- Logs each attempt with attempt number and error details
- Returns HTML string on success, throws after retries exhausted
stephen changed target branch from main to dev 2026-01-31 11:07:59 -07:00
stephen commented 2026-02-05 15:35:30 -07:00
Author
Owner
Copy Link

❌ Changes Requested

Two issues: (1) Deleted scraperConfig.test.js removes 192 lines of config test coverage with no justification. (2) Deploy workflow change from github.head_ref || github.ref_name to github.ref_name will break branch name resolution for PR-triggered webhook payloads.

Issues:

tests/scraper/scraperConfig.test.js:0
This test file (192 lines) was deleted entirely. These tests validate the scraper config module's defaults, environment variable overrides, and type coercion. Even though the config changes were minor (comment cleanup, parseInt radix), removing the tests reduces coverage. Please restore this file or explain why these tests are no longer needed.

.github/workflows/deploy.yml:131
Changed from github.head_ref || github.ref_name to github.ref_name. For pull_request events, github.ref_name resolves to the merge ref (e.g., refs/pull/8/merge) rather than the source branch name. The original pattern is correct: head_ref provides the source branch on PRs, ref_name is the fallback for push events. This change will send incorrect branch names in the webhook payload for PR builds. Please revert this line.

## ❌ Changes Requested Two issues: (1) Deleted scraperConfig.test.js removes 192 lines of config test coverage with no justification. (2) Deploy workflow change from `github.head_ref || github.ref_name` to `github.ref_name` will break branch name resolution for PR-triggered webhook payloads. ### Issues: **__tests__/scraper/scraperConfig.test.js:0** This test file (192 lines) was deleted entirely. These tests validate the scraper config module's defaults, environment variable overrides, and type coercion. Even though the config changes were minor (comment cleanup, parseInt radix), removing the tests reduces coverage. Please restore this file or explain why these tests are no longer needed. **.github/workflows/deploy.yml:131** Changed from `github.head_ref || github.ref_name` to `github.ref_name`. For pull_request events, `github.ref_name` resolves to the merge ref (e.g., `refs/pull/8/merge`) rather than the source branch name. The original pattern is correct: `head_ref` provides the source branch on PRs, `ref_name` is the fallback for push events. This change will send incorrect branch names in the webhook payload for PR builds. Please revert this line.
stephen force-pushed scraper/fetch-page from 19baf43493 to d09aa1d179 2026-02-05 15:46:09 -07:00 Compare
stephen commented 2026-02-05 15:52:38 -07:00
Author
Owner
Copy Link

Root Cause

The branch was created before SCRAPE-2 (PR #7) was merged into dev. The PR diff showed SCRAPE-2 changes as "deleted" because the branch didn't include them.

Resolution

  • scraperConfig.test.js: No longer appears in PR diff — the file is now in the branch's history from the SCRAPE-2 merge
  • deploy.yml: No longer appears in PR diff — the github.head_ref || github.ref_name fix is preserved from SCRAPE-2
  • config/scraper.js conflict: Resolved by keeping SCRAPE-2's header/comments with the fetch-page branch's parseInt radix parameter improvement

Verification

  • All 26 fetchPage tests pass
  • All 25 scraperConfig tests pass
  • PR diff now only shows the actual fetchPage changes (3 files: fetchPage.test.js, scraperService.js, config/scraper.js parseInt radix fix)")
    +-------+--------------------------------+-------+---------+-----------+------------------+--------+
    | INDEX | TITLE | STATE | AUTHOR | MILESTONE | UPDATED | LABELS |
    +-------+--------------------------------+-------+---------+-----------+------------------+--------+
    | 8 | SCRAPE-3: Implement | open | stephen | | 2026-02-05 15:46 | |
    | | fetchPage() with axios, | | | | | |
    | | timeout, retry | | | | | |
    +-------+--------------------------------+-------+---------+-----------+------------------+--------+
#### Root Cause The branch was created before SCRAPE-2 (PR #7) was merged into dev. The PR diff showed SCRAPE-2 changes as "deleted" because the branch didn't include them. #### Resolution - **scraperConfig.test.js:** No longer appears in PR diff — the file is now in the branch's history from the SCRAPE-2 merge - **deploy.yml:** No longer appears in PR diff — the `github.head_ref || github.ref_name` fix is preserved from SCRAPE-2 - **config/scraper.js conflict:** Resolved by keeping SCRAPE-2's header/comments with the fetch-page branch's `parseInt` radix parameter improvement #### Verification - All 26 fetchPage tests pass - All 25 scraperConfig tests pass - PR diff now only shows the actual fetchPage changes (3 files: fetchPage.test.js, scraperService.js, config/scraper.js parseInt radix fix)") +-------+--------------------------------+-------+---------+-----------+------------------+--------+ | INDEX | TITLE | STATE | AUTHOR | MILESTONE | UPDATED | LABELS | +-------+--------------------------------+-------+---------+-----------+------------------+--------+ | 8 | SCRAPE-3: Implement | open | stephen | | 2026-02-05 15:46 | | | | fetchPage() with axios, | | | | | | | | timeout, retry | | | | | | +-------+--------------------------------+-------+---------+-----------+------------------+--------+
stephen merged commit c6beaf8333 into dev 2026-02-05 15:58:08 -07:00
stephen deleted branch scraper/fetch-page 2026-02-05 15:58:08 -07:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
stephen
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: stephen/apartment-dashboard-api#8
No description provided.
Delete Branch "scraper/fetch-page"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?