[REFAC#75] goquery/chromedp 크롤러 중복 패턴 공통 추출 - #123
Conversation
There was a problem hiding this comment.
Code Review
This pull request centralizes crawler logic by introducing a unified HTTP status code checker and a common constructor for RawContent objects, refactoring the chromedp and goquery implementations to use these shared utilities. The review feedback highlights a potential issue with the ID generation logic in NewRawContent, which uses UnixNano() and may lead to collisions in high-concurrency scenarios, as indicated by the reliance on time.Sleep in the associated tests.
There was a problem hiding this comment.
Pull request overview
goquery/chromedp fetcher에 중복되어 있던 HTTP 상태코드 분기와 RawContent 조립 로직을 internal/crawler/core의 공통 헬퍼로 추출하여, 정책 변경을 단일 지점(SoT)에서 관리하도록 리팩토링한 PR입니다.
Changes:
core.CheckHTTPStatus(url, statusCode)추가로 4xx/5xx 분기 로직을 공통화core.NewRawContent(...)추가로 RawContent 생성 패턴을 공통화(헤더 nil 보정 포함)- goquery/chromedp fetcher에 헬퍼 적용 + 신규 단위 테스트(HTTP status 16, RawContent 6) 추가
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/crawler/core/http_status.go |
HTTP 상태코드 → CrawlerError 변환 분기 로직을 공통 함수로 추출 |
internal/crawler/core/models.go |
RawContent 생성자(NewRawContent) 추가로 조립 로직 일원화 |
internal/crawler/implementation/goquery/fetch.go |
상태코드 분기/RawContent 조립 중복 제거 후 core 헬퍼 사용 |
internal/crawler/implementation/chromedp/fetch.go |
상태코드 분기/RawContent 조립 중복 제거 후 core 헬퍼 사용(부분 로드 metadata 정책 유지) |
test/internal/crawler_core/http_status_test.go |
CheckHTTPStatus 분기 정책 회귀 테스트 추가 |
test/internal/crawler_core/raw_content_builder_test.go |
NewRawContent 필드 대입/headers nil 보정/metadata 참조 정책 테스트 추가 |
기존: goquery/fetch.go, chromedp/fetch.go 두 곳에 동일한 4-way 분기 블록 (404 / 429 / >=500 / >=400) 이 중복. 신규: core.CheckHTTPStatus(url, code) error — 단일 진실 소스(SoT) 로 분기 정책 일원화. - 404 → NewNotFoundError (retry 불가) - 429 → NewRateLimitError (retry 가능) - 5xx → NewHTTPServerError (retry 가능) - 4xx → NewHTTPClientError (retry 불가) - 그 외 → nil 새 분기(예: 451) 추가 시 본 함수만 수정하면 모든 fetcher 에 일관 반영. chromedp 의 status==0 같은 fetcher 고유 sentinel 은 호출자가 사전 보정 후 호출. 테스트 16건 (table-driven 14 케이스 + 경계값 2건): - 200/204/301/399 → nil - 400/401/403/499 → HTTP_4xx (Internal, retry=false) - 404 → HTTP_404 (NotFound, retry=false) - 429 → HTTP_429 (RateLimit, retry=true) - 500/502/503/599 → HTTP_5xx (Network, retry=true) - 399→400 / 499→500 경계에서 분기 정확 전환 본 commit 은 helper 만 추가, 적용은 후속 commit.
기존: goquery/fetch.go, chromedp/fetch.go 두 곳에서 8 필드 동일 조립. 신규: NewRawContent(name, source, target, html, statusCode, headers) *RawContent - ID 형식: '<name>-<unix_nano>' (호출 시점 고유성) - target.URL / target.Metadata 자동 추출 (target 통째 수신) - nil headers 는 빈 map 으로 보정 (downstream nil dereference 방지) - Metadata reference 그대로 (chromedp partial_load 같은 변형은 호출자 책임) 테스트 6건: - 8 필드 모두 정확 대입 - nil headers → 빈 map 보정 + write 가능 - ID format prefix 검증 (다른 fetcher 다른 prefix) - 연속 호출 100회 ID 중복 없음 - nil metadata 보존 - Metadata reference 미복사 (호출자 변형 가능) 본 commit 은 helper 만 추가, 적용은 후속 commit.
중복 블록 2개 제거: 1. HTTP 상태코드 4-way 분기 (12줄) → core.CheckHTTPStatus(url, code) 1줄 호출 2. RawContent 8-필드 조립 + Headers populating (15줄) → core.NewRawContent(...) 1줄 + Headers 추출 loop (5줄, fetcher 고유) 부수 효과: - import 'fmt' / 'time' 제거 (NewRawContent 가 흡수) - Headers map 사전 capacity 명시 (len(resp.Header) — 작은 최적화) 동작 동일성 검증: 전체 회귀 테스트 통과.
중복 블록 2개 제거:
1. HTTP 상태코드 4-way 분기 (12줄) → core.CheckHTTPStatus(url, code) 1줄
2. RawContent 8-필드 조립 (9줄) → core.NewRawContent(...) 1줄
+ partialLoad 시 metadata 덮어쓰기 (chromedp 고유 변형, 4줄 → 3줄)
설계 결정:
- chromedp 는 raw HTTP header 미접근 → NewRawContent 에 nil headers 전달
→ 생성자가 빈 map 으로 보정
- partial_load 메타데이터 변형은 NewRawContent 호출 후 raw.Metadata 덮어쓰기
→ 생성자는 단순 대입 정책 유지 (variant 정책은 호출자 책임)
- capturedStatus==0 fallback 분기는 chromedp 고유 → CheckHTTPStatus 호출 전 그대로 유지
동작 동일성 검증: 전체 회귀 테스트 통과 (race detector 포함).
기존 ID 형식 '<name>-<unix_nano>' 는 동일 nanosecond 다중 호출 시 충돌 가능 —
테스트가 sleep(1µs) 로 우회한 점이 그 취약성의 증거 (gemini 지적).
신규 형식 '<name>-<unix_nano>-<rand_hex>':
- <unix_nano> : 시간 정렬·디버깅 추적성 보존
- <rand_hex> : crypto/rand 4바이트 (8자 hex) suffix → 1/2^32 per ns 충돌
- rand.Read 실패 시 0-suffix fallback (fetch 차단 방지)
- newRawContentID(name) 헬퍼로 분리 (단일 책임)
테스트 강화:
- ConsecutiveCalls: sleep 제거 + 1000회 (이전 100회 + sleep)
- HighConcurrency 신규: 50 goroutine × 100회 = 5000건 동시 호출 충돌 0
(-race detector 로 동시 검증)
기존: time.Now() 가 ID 생성과 FetchedAt 에서 각각 호출 (2 syscall) → ns 단위 미세 불일치 + 디버깅 시 ID timestamp ≠ FetchedAt 가능성 수정: - now := time.Now() 단일 호출 - newRawContentID 시그니처에 time.Time 파라미터 추가 → 호출자가 동일 시점 전달 - ID 의 unix_nano 부분과 FetchedAt 이 정확히 일치 이점: - 디버깅 일관성 (ID의 시간 부분 = FetchedAt) - syscall 1회 절감 (미세 성능)
Agent-Logs-Url: https://github.com/EinSofINTEREST/IssueTracker/sessions/f8ea93ff-333b-4db9-bd64-5e0d1ef2377c Co-authored-by: juhy0987 <58243998+juhy0987@users.noreply.github.com>
연관 이슈
구현 내용
신규 `core` 헬퍼 2개 (commit 1, 2)
`core.CheckHTTPStatus(url, statusCode) error`
`core.NewRawContent(name, source, target, html, statusCode, headers) *RawContent`
Fetcher 적용 (commit 3, 4)
goquery/fetch.go — 27줄 삭제, 9줄 추가
chromedp/fetch.go — 26줄 삭제, 10줄 추가
테스트 — 22 신규 (모두 PASS)
전체 회귀 테스트 통과 (`go test -race ./...`).
CI / 머지 게이트 점검
변경 영향 범위
Required Status Checks
롤백 계획
TODO
논의 사항