Skip to content

Conversation

@cnlangzi
Copy link
Owner

@cnlangzi cnlangzi commented Jan 11, 2026

Summary by Sourcery

Bug Fixes:

  • Treat NXDOMAIN (missing PTR record) during reverse DNS lookup as a hard verification failure instead of a retryable network error.

- Use *net.DNSError.IsNotFound to detect NXDOMAIN (no PTR record)
- NXDOMAIN now returns StatusFailed instead of StatusPending
- Network errors (timeout, connection issues) still return StatusPending for retry
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 11, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Handle reverse DNS lookup errors by treating NXDOMAIN (no PTR record) as a hard failure while keeping other network errors as retriable.

Sequence diagram for RDNS verification with NXDOMAIN handling

sequenceDiagram
  participant Bot
  participant Net as net

  Bot->>Net: LookupAddr(ipStr)
  Net-->>Bot: error
  alt error is net.DNSError with IsNotFound true
    Bot->>Bot: fail.Add(ipStr)
    Bot-->>Bot: return StatusFailed
  else other network error
    Bot-->>Bot: return StatusPending
  end
Loading

File-Level Changes

Change Details Files
Differentiate NXDOMAIN from other network errors in reverse DNS verification and update status/fail-cache behavior accordingly.
  • Inspect net.LookupAddr error for *net.DNSError and check IsNotFound to detect NXDOMAIN (no PTR record).
  • On NXDOMAIN, mark the IP as failed via b.fail.Add(ipStr) and return StatusFailed to indicate a hard failure.
  • For all other lookup errors (timeouts, transient network issues), continue returning StatusPending to allow retries.
bot.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider using errors.As instead of a direct type assertion to *net.DNSError so that wrapped DNS errors are still correctly classified as NXDOMAIN vs network issues.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider using `errors.As` instead of a direct type assertion to `*net.DNSError` so that wrapped DNS errors are still correctly classified as NXDOMAIN vs network issues.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link

codecov bot commented Jan 11, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.56%. Comparing base (60ce193) to head (c9cd856).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
bot.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
- Coverage   72.76%   71.56%   -1.21%     
==========================================
  Files          15       17       +2     
  Lines         661      735      +74     
==========================================
+ Hits          481      526      +45     
- Misses        136      155      +19     
- Partials       44       54      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cnlangzi cnlangzi merged commit 18bcefc into main Jan 11, 2026
3 of 5 checks passed
@cnlangzi cnlangzi deleted the fix/rdns branch January 11, 2026 14:14
@github-actions
Copy link

Benchmark Results

BenchmarkFindBotByUA_Hit_First             	  774824	      1714 ns/op	      21 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_First-4           	 2457927	       694.6 ns/op	      17 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_First-8           	 3359643	       612.4 ns/op	      15 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Middle            	 1427029	       877.4 ns/op	      13 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Middle-4          	 4100294	       499.8 ns/op	      13 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Middle-8          	 5023794	       333.2 ns/op	       8 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Last              	 1408330	      1207 ns/op	      19 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Last-4            	 2709805	       397.3 ns/op	      11 B/op	       0 allocs/op
BenchmarkFindBotByUA_Hit_Last-8            	--- FAIL: BenchmarkFindBotByUA_Hit_Last-8
BenchmarkFindBotByUA_Miss                  	  507398	      2635 ns/op	      95 B/op	       0 allocs/op
BenchmarkFindBotByUA_Miss-4                	 1363532	       858.5 ns/op	      21 B/op	       0 allocs/op
BenchmarkFindBotByUA_Miss-8                	 1342603	       901.3 ns/op	      25 B/op	       0 allocs/op
BenchmarkFindBotByUA_CaseSensitive         	 1000000	      1020 ns/op	      13 B/op	       0 allocs/op
BenchmarkFindBotByUA_CaseSensitive-4       	 3121101	       444.3 ns/op	      12 B/op	       0 allocs/op
BenchmarkFindBotByUA_CaseSensitive-8       	 4787684	       307.5 ns/op	       6 B/op	       0 allocs/op
BenchmarkValidate_KnownBot_IPHit           	 1000000	      1092 ns/op	      23 B/op	       0 allocs/op
BenchmarkValidate_KnownBot_IPHit-4         	 2200286	       560.4 ns/op	      14 B/op	       0 allocs/op
BenchmarkValidate_KnownBot_IPHit-8         	 3134862	       526.5 ns/op	      20 B/op	       0 allocs/op
BenchmarkValidate_Browser                  	--- FAIL: BenchmarkValidate_Browser
BenchmarkValidate_Browser-4                	  627133	      2057 ns/op	      77 B/op	       0 allocs/op
BenchmarkValidate_Browser-8                	  498900	      2026 ns/op	      79 B/op	       0 allocs/op
BenchmarkContainsWord                      	72368844	        18.29 ns/op	       0 B/op	       0 allocs/op
BenchmarkContainsWord-4                    	73669558	        16.87 ns/op	       0 B/op	       0 allocs/op
BenchmarkContainsWord-8                    	73214562	        16.20 ns/op	       0 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA                	  952285	      1165 ns/op	       9 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA-4              	 2747359	       440.8 ns/op	      10 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA-8              	 2728572	       451.9 ns/op	       4 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA_IPMismatch     	  773910	      1979 ns/op	      65 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA_IPMismatch-4   	 2018013	       577.5 ns/op	      18 B/op	       0 allocs/op
BenchmarkValidate_WithBotUA_IPMismatch-8   	 2027438	       571.5 ns/op	      10 B/op	       0 allocs/op
BenchmarkValidate_BrowserUA                	  287529	      4971 ns/op	     200 B/op	       1 allocs/op
BenchmarkValidate_BrowserUA-4              	--- FAIL: BenchmarkValidate_BrowserUA-4
BenchmarkValidate_BrowserUA-8              	  854090	      1521 ns/op	      56 B/op	       0 allocs/op
BenchmarkValidate_UnknownBotUA             	 8143225	       186.2 ns/op	       7 B/op	       0 allocs/op
BenchmarkValidate_UnknownBotUA-4           	21948859	        60.53 ns/op	       3 B/op	       0 allocs/op
BenchmarkValidate_UnknownBotUA-8           	22563117	        56.80 ns/op	       1 B/op	       0 allocs/op
BenchmarkContainsIP                        	55834134	      1962 ns/op	       1 B/op	       0 allocs/op
BenchmarkContainsIP-4                      	94331160	        11.27 ns/op	       0 B/op	       0 allocs/op
BenchmarkContainsIP-8                      	100000000	        11.56 ns/op	       0 B/op	       0 allocs/op
BenchmarkFindBotByUA                       	  795450	      1943 ns/op	      73 B/op	       0 allocs/op
BenchmarkFindBotByUA-4                     	 2041138	       607.5 ns/op	      21 B/op	       0 allocs/op
BenchmarkFindBotByUA-8                     	 2034795	       578.4 ns/op	      17 B/op	       0 allocs/op
BenchmarkClassifyUA                        	 2052313	       550.0 ns/op	       4 B/op	       0 allocs/op
BenchmarkClassifyUA-4                      	SIGQUIT: quit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants