Feat: Add Host IP address in the overview and sharable

[FarshidRoohi]

📷 Screenshots

📄 Context

I needed to know the IP address of each request because each request might be directed to a specific server with a specific IP address. Having the CDN IP address can be very helpful in troubleshooting issues. Therefore, I thought it would be useful for other developers as well, and that's why I added this information.

📝 Changes

• Added a new field on HttpTransaction
• Increased Room DB version from 9 to 10

[cortinico]

Thanks for the PR @FarshidRoohi.
I've left one comment

[cortinico]

Can you test this function?

[FarshidRoohi]

I wrote two tests for this extension function.
https://github.com/ChuckerTeam/chucker/pull/1180/files#diff-9e02dc5b49faba047a35920cbd28850dec0ae4a568dd09ba6264dc754c332dcaR55

[MiSikora]

I'm having trouble understanding this. Why is it assumed that the response body contains IP?

[FarshidRoohi]

It's a bad way. I fixed it.

[MiSikora]

This reads the whole body of the source to the memory and assumes that it is UTF-8 encoded.

[FarshidRoohi]

I deleted this function.

[ibrahim-iqbal]

Read through carefully — the feature is genuinely useful (Chucker shows an OkHttp transaction's target host, but not the IP the socket actually reached), but the current implementation has three problems that would need to be fixed before it can merge.

1. getHostIp() reads the wrong thing entirely.

public fun BufferedSource.getHostIp(): String? {
    val body = readUtf8()
    val pattern: Pattern = Pattern.compile(IP_REGEX)
    ...
}

This scans the response body for the first d.d.d.d substring. That is not the host IP — it's any IP-shaped text the server happened to write into its response (a JSON remote_addr field, a Content-Security-Policy header echoed into HTML, a copyright line with a version number, etc.). For most JSON APIs it returns null; for HTML it returns whatever appears first, often a totally unrelated IP.

The actual host IP is only available from the socket. Inside an OkHttp interceptor:

val hostIp = chain.connection()?.socket()?.inetAddress?.hostAddress

This is what OkHttp itself exposes in EventListener.connectStart/connectEnd, and it's what will match the value users see in nslookup / dig / tcpdump. It also covers IPv6 for free.

2. readUtf8() consumes the BufferedSource.

Even if the regex approach were correct, calling readUtf8() on BufferedSource drains it — the body becomes empty for every subsequent reader in the chain, including Chucker's own body-recording code and the caller who owns the response. Either peek().readUtf8() (reads without consuming) or reading from a saved buffer would be required. Moving the lookup to the socket (per #1) sidesteps this entirely.

3. Room schema bump 9 → 10 needs a migration.

-@Database(entities = [HttpTransaction::class], version = 9, exportSchema = false)
+@Database(entities = [HttpTransaction::class], version = 10, exportSchema = false)

Adding the hostIp column bumps the version, but there is no Migration provided and no fallbackToDestructiveMigration() on the Room.databaseBuilder. On upgrade, users will hit IllegalStateException: A migration from 9 to 10 was required but not found and either lose their transaction history (with the fallback) or crash outright. A one-line ALTER TABLE ... ADD COLUMN hostIp TEXT migration handles it.

Smaller notes:

• IP_REGEX = "(?:\\d{1,3}\\.){3}\\d{1,3}" matches strings like 999.999.999.999 too, and doesn't cover IPv6. Socket-based lookup avoids the parsing question.
• If the field is only shown in the overview and shared payload, computing it lazily from the transaction's cached connection info (rather than persisting it) would let old rows in the DB stay valid without a migration.

Great direction, though — Chucker showing which IP the request actually reached is real diagnostic value, especially for CDN-routed traffic where the host header hides that.

[FarshidRoohi]

Hi @cortinico, @MiSikora, and @ibrahim-iqbal — thank you for your reviews and detailed feedback.

I’ve addressed the review items:

• The connected peer IP is now obtained from OkHttp’s connection route instead of parsing the response body.
• The response-body parsing function was removed, so the body is no longer consumed and IPv6 is supported automatically.
• The database version was updated, following the project’s existing destructive-migration policy.
• Integration coverage was added to verify that the peer IP is available to network interceptors and remains unavailable to application interceptors.
• The README and changelog were updated to document this behavior and its limitations.

Could you please take another look when you have a chance? Thanks!

[ibrahim-iqbal]

Re-reviewed after the latest round.

The route-based approach is correct — chain.connection()?.route()?.socketAddress?.address?.hostAddress handles IPv4 and IPv6 uniformly through InetAddress.getHostAddress(), and the parameterized connected peer IP is available to network interceptors only test proves the app-vs-network contract in one shot. Stubbing connection() returns null in ChuckerInterceptorSkipRequestTest matches what happens on skipped requests and on responses served from the OkHttp cache.

Two UX polish items on the null path, both non-blocking:

1. chucker_fragment_transaction_overview.xml and TransactionOverviewFragment.kt show the "Host IP" label unconditionally. When Chucker is registered as an application interceptor (or the response came from the cache), transaction.hostIp is null and the value TextView renders empty while the label still shows. Consider gating both TextViews to View.GONE when hostIp == null, so app-interceptor users don't see an orphan label. The README tip already explains why it can be empty, but hiding the row reads better than an empty one.

2. TransactionDetailsSharable writes Host IP: null\n verbatim when hostIp is null. Skipping the line (or substituting an em-dash) would avoid a literal null showing up in shared debug logs.

Migration bump to version = 11 is consistent with prior column-only additions in this repo, and HttpTransactionDaoTest covers hostIp alongside the existing columns.

[cortinico]

LGTM 👍