Skip to content

Expose DNS error codes through the UnknownHostException#13721

Merged
normanmaurer merged 2 commits intonetty:4.1from
tkountis:netty-dns-error-cause
Jan 12, 2024
Merged

Expose DNS error codes through the UnknownHostException#13721
normanmaurer merged 2 commits intonetty:4.1from
tkountis:netty-dns-error-cause

Conversation

@tkountis
Copy link
Copy Markdown
Contributor

@tkountis tkountis commented Dec 8, 2023

Motivation:

UnknownHostException are covering a broader range of discovery failures, where the API consumer has no visibility of the underlying reason.

Modification:

Expose initial-cause for UnknownHostExceptions to enrich the result.

Result:

The consumer now can decide whether the exception is due to failures or due an authoritative NXDOMAIN.

Comment thread resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java Outdated
Comment thread resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java Outdated
@tkountis tkountis force-pushed the netty-dns-error-cause branch from b7331bf to 79c4988 Compare December 9, 2023 00:35
@tkountis tkountis marked this pull request as ready for review December 9, 2023 00:39
@tkountis tkountis changed the title [WIP] Expose DNS error code through the UnknownHostException when not NX Expose DNS error codes through the UnknownHostException Dec 9, 2023
@normanmaurer
Copy link
Copy Markdown
Member

@tkountis I did retry this build a few times now and it always fails during dns tests… Can you have a look if this might break something ?

@tkountis tkountis force-pushed the netty-dns-error-cause branch from fadbeec to 4e07483 Compare January 12, 2024 01:47
@tkountis tkountis requested a review from normanmaurer January 12, 2024 01:48
@normanmaurer normanmaurer added this to the 4.1.105.Final milestone Jan 12, 2024
@normanmaurer normanmaurer merged commit 47e1f43 into netty:4.1 Jan 12, 2024
@normanmaurer
Copy link
Copy Markdown
Member

@tkountis thanks a lot!

normanmaurer pushed a commit that referenced this pull request Jan 12, 2024
Motivation:

UnknownHostException are covering a broader range of discovery failures,
where the API consumer has no visibility of the underlying reason.

Modification:

Expose initial-cause for UnknownHostExceptions to enrich the result.

Result:

The consumer now can decide whether the exception is due to failures or
due an authoritative NXDOMAIN.
franz1981 pushed a commit to franz1981/netty that referenced this pull request Feb 9, 2024
Motivation:

UnknownHostException are covering a broader range of discovery failures,
where the API consumer has no visibility of the underlying reason.

Modification:

Expose initial-cause for UnknownHostExceptions to enrich the result. 

Result:

The consumer now can decide whether the exception is due to failures or
due an authoritative NXDOMAIN.
normanmaurer added a commit that referenced this pull request Feb 21, 2024
Follow up of #13721

Motivation:

The previous effort introduced a root-cause for NXDOMAIN and SERVFAIL.
That was only for the authoritative part. The non-authoritative was
slightly different because it allowed a CNAME lookup regardless, for
legacy reasons. In this effort, I take another stab at it, by making the
additional CNAME lookup an opt-in feature. Justification included in the
patch.

Modification:

Last resort CNAME lookup while doing address resolutions, is now an
opt-in feature by default. The non-authoritative flow also includes the
root cause as part of the UnknownHostException.

---------

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
normanmaurer added a commit that referenced this pull request Feb 21, 2024
Follow up of #13721

Motivation:

The previous effort introduced a root-cause for NXDOMAIN and SERVFAIL.
That was only for the authoritative part. The non-authoritative was
slightly different because it allowed a CNAME lookup regardless, for
legacy reasons. In this effort, I take another stab at it, by making the
additional CNAME lookup an opt-in feature. Justification included in the
patch.

Modification:

Last resort CNAME lookup while doing address resolutions, is now an
opt-in feature by default. The non-authoritative flow also includes the
root cause as part of the UnknownHostException.

---------

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
jrhee17 added a commit to line/armeria that referenced this pull request Apr 11, 2024
**Dependencies**

- Brave 6.0.0 -> 6.0.2
- Brotli4j 1.15.0 -> 1.16.0
- java-control-plane 1.0.42 -> 1.0.44
- Dropwizard 2.1.10 -> 2.1.12
- Dropwizard Metrics 4.2.24 -> 4.2.25
- Eureka 2.0.1 -> 2.0.2
- fastUtil 8.5.12 -> 8.5.13
- gRPC-Java 1.61.0 -> 1.63.0
- Guava 33.0.0-jre -> 33.1.0-jre
- Jackson 2.16.1 -> 2.17.0
- JCTools 4.0.2 -> 4.0.3
- Jetty10 10.0.19 -> 10.0.20
- Jetty11 11.0.19 -> 11.0.20
- Jetty12 12.0.5 -> 12.0.8
- Jetty9.4 9.4.52.v20230823 -> 9.4.54.v20240208
- Kotlin 1.9.22 -> 1.9.23
- kotlinx.coroutines 1.9.22 -> 1.9.23
- Kubernetes Client 6.10.0 -> 6.11.0
- Micrometer 1.12.2 -> 1.12.4
- Micrometer Tracing 1.2.2 -> 1.2.4
- Netty 4.1.106.Final -> 4.1.108.Final
- Netty io_uring 0.0.24.Final -> 0.0.25.Final
- protobuf-jackson 2.2.0 -> 2.5.0
- Reactor 3.6.2 -> 3.6.4
- RESTEasy 5.0.7.Final -> 5.0.9.Final
- Retrofit2 2.9.0 -> 2.11.0
- Sangria 4.0.2 -> 4.1.0
- Scala2.12 2.12.18 -> 2.12.19
- Scala2.13 2.13.12 -> 2.13.13
- Scala3 3.3.0 -> 3.4.1
- Spring6 6.1.3 -> 6.1.5
- Spring-Boot3 3.2.2 -> 3.2.4
- Tomcat8 8.5.98 -> 8.5.100
- Tomcat8 9.0.85 -> 9.0.87
- Tomcat8 10.1.18 -> 10.1.20
- Build
  - Apache HttpClient 5.3 -> 5.3.1
  - asm 9.6 -> 9.7
  - AssertJ 3.25.2 -> 3.25.3
  - Awaitility 4.2.1 -> 4.2.1
  - dagger 2.50 -> 2.51.1
  - dgs 8.2.2 -> 8.5.3
  - Error Prone 2.24.1 -> 2.26.1
  - GAX Java 2.40.0 -> 2.46.1
  - Gradle 8.5 -> 8.7
  - J2ObjC 2.8 -> 3.0.0
  - Java-WebSocket 1.5.5 -> 1.5.6
  - JUnit5 5.10.1 -> 5.10.2
  - Kafka 3.6.1 -> 3.7.0
  - krotoDC 1.0.6 -> 1.1.1
  - Gradle Nexus Publish Plugin 7.0.1 -> 7.0.2
  - SLF4J2 2.0.11 -> 2.0.12
  - Testcontainers 1.19.3 -> 1.19.7
  - Zookeeper 3.9.1 -> 3.9.2

**Additional Modifications**
- `closeAndReleaseStagingRepository` has been renamed to
`closeAndReleaseStagingRepositories`
  - gradle-nexus/publish-plugin#236
- `DnsErrorCauseException` has been introduced in Netty
  - netty/netty#13721
  - `DnsUtil#isDnsQueryTimedOut` has been updated to reflect this
- `RefreshingAddressResolver` now uses `DnsUtil#isDnsQueryTimedOut` to
determine cachability for consistency
- Note that users should sync versions between netty and armeria for
this release. Otherwise, a compliation error may occur.
- `krotodc` now requires the `protobuf-java-util` dependency
  - mscheong01/krotoDC#25
- Sangria has a breaking change regarding `DeprecationTracker`
  - sangria-graphql/sangria#1064
tkountis added a commit to apple/servicetalk that referenced this pull request Apr 11, 2024
Motivation:
An UnknownHostException can have multiple root causes, each with a different application logic.
By far the most prominent is due to an NXDOMAIN from the nameserver, meaning that the domain in the query doesn't not exist. Other reasons could be a SERVFAIL or timeouts during resolutions. Due to insufficient information in the error itself, the dns-client has no obvious way of distinctly handling the various causes.

Modification:
Netty will expose that information as an init-cause. ST can leverage the enrichment to handle the different scenarios.

Results:
Better behavior.
To allow NXDOMAIN errors to invalidate DNS state, set the property `io.servicetalk.dns.discovery.nxdomain.invalidation` to true.

Depends on netty/netty#13721
Depends on netty/netty#13850
Dogacel pushed a commit to Dogacel/armeria that referenced this pull request Jun 8, 2024
**Dependencies**

- Brave 6.0.0 -> 6.0.2
- Brotli4j 1.15.0 -> 1.16.0
- java-control-plane 1.0.42 -> 1.0.44
- Dropwizard 2.1.10 -> 2.1.12
- Dropwizard Metrics 4.2.24 -> 4.2.25
- Eureka 2.0.1 -> 2.0.2
- fastUtil 8.5.12 -> 8.5.13
- gRPC-Java 1.61.0 -> 1.63.0
- Guava 33.0.0-jre -> 33.1.0-jre
- Jackson 2.16.1 -> 2.17.0
- JCTools 4.0.2 -> 4.0.3
- Jetty10 10.0.19 -> 10.0.20
- Jetty11 11.0.19 -> 11.0.20
- Jetty12 12.0.5 -> 12.0.8
- Jetty9.4 9.4.52.v20230823 -> 9.4.54.v20240208
- Kotlin 1.9.22 -> 1.9.23
- kotlinx.coroutines 1.9.22 -> 1.9.23
- Kubernetes Client 6.10.0 -> 6.11.0
- Micrometer 1.12.2 -> 1.12.4
- Micrometer Tracing 1.2.2 -> 1.2.4
- Netty 4.1.106.Final -> 4.1.108.Final
- Netty io_uring 0.0.24.Final -> 0.0.25.Final
- protobuf-jackson 2.2.0 -> 2.5.0
- Reactor 3.6.2 -> 3.6.4
- RESTEasy 5.0.7.Final -> 5.0.9.Final
- Retrofit2 2.9.0 -> 2.11.0
- Sangria 4.0.2 -> 4.1.0
- Scala2.12 2.12.18 -> 2.12.19
- Scala2.13 2.13.12 -> 2.13.13
- Scala3 3.3.0 -> 3.4.1
- Spring6 6.1.3 -> 6.1.5
- Spring-Boot3 3.2.2 -> 3.2.4
- Tomcat8 8.5.98 -> 8.5.100
- Tomcat8 9.0.85 -> 9.0.87
- Tomcat8 10.1.18 -> 10.1.20
- Build
  - Apache HttpClient 5.3 -> 5.3.1
  - asm 9.6 -> 9.7
  - AssertJ 3.25.2 -> 3.25.3
  - Awaitility 4.2.1 -> 4.2.1
  - dagger 2.50 -> 2.51.1
  - dgs 8.2.2 -> 8.5.3
  - Error Prone 2.24.1 -> 2.26.1
  - GAX Java 2.40.0 -> 2.46.1
  - Gradle 8.5 -> 8.7
  - J2ObjC 2.8 -> 3.0.0
  - Java-WebSocket 1.5.5 -> 1.5.6
  - JUnit5 5.10.1 -> 5.10.2
  - Kafka 3.6.1 -> 3.7.0
  - krotoDC 1.0.6 -> 1.1.1
  - Gradle Nexus Publish Plugin 7.0.1 -> 7.0.2
  - SLF4J2 2.0.11 -> 2.0.12
  - Testcontainers 1.19.3 -> 1.19.7
  - Zookeeper 3.9.1 -> 3.9.2

**Additional Modifications**
- `closeAndReleaseStagingRepository` has been renamed to
`closeAndReleaseStagingRepositories`
  - gradle-nexus/publish-plugin#236
- `DnsErrorCauseException` has been introduced in Netty
  - netty/netty#13721
  - `DnsUtil#isDnsQueryTimedOut` has been updated to reflect this
- `RefreshingAddressResolver` now uses `DnsUtil#isDnsQueryTimedOut` to
determine cachability for consistency
- Note that users should sync versions between netty and armeria for
this release. Otherwise, a compliation error may occur.
- `krotodc` now requires the `protobuf-java-util` dependency
  - mscheong01/krotoDC#25
- Sangria has a breaking change regarding `DeprecationTracker`
  - sangria-graphql/sangria#1064
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.

4 participants