Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .github/workflows/rerun-flaky.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: rerun-flaky

# Re-runs the emulator suite once when it fails, and nothing else.
#
# `android-ech` fails on infrastructure often enough to be a nuisance: the emulator comes up
# half-dead, `package install-create` answers `Can't find service: package`, and the job ends
# having run *zero* tests. That is not a result about OkHttp, and re-running it by hand is the
# only thing anyone does about it.
#
# Deliberately narrow, because an auto-retry is a way to hide real failures:
#
# - Only `android-ech`. The container and network suites fail for reasons worth reading, and
# a flaky emulator is a problem the others do not have. Extending this to them would mean
# deciding their failures are noise too, which is a different and much bigger claim.
# - Only once, guarded on `run_attempt == 1`. A second failure is the answer, not an
# invitation to keep asking: something that fails twice is either broken or flaky enough to
# be worth fixing rather than absorbing.
# - The re-run is visible. It appears as attempt 2 of the same run, so the history shows both
# the failure and the retry rather than quietly presenting a green first attempt.
#
# If this workflow starts firing regularly, that is the signal to fix the emulator setup — not
# to widen the retry.
on:
workflow_run:
workflows: [android-ech]
types: [completed]

permissions:
actions: write

jobs:
rerun:
if: >-
github.event.workflow_run.conclusion == 'failure' &&
github.event.workflow_run.run_attempt == 1
runs-on: ubuntu-latest
steps:
- name: Re-run the failed jobs once
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GH_REPO: ${{ github.repository }}
RUN_ID: ${{ github.event.workflow_run.id }}
run: |
echo "Re-running failed jobs in run $RUN_ID (attempt ${{ github.event.workflow_run.run_attempt }})"
gh run rerun "$RUN_ID" --failed
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,23 @@ It loads badssl's published PKCS#12 through a `KeyStore` and a `KeyManagerFactor
`okhttp-tls`: `HandshakeCertificates` has no route in from a PKCS#12, and converting it first
would mean testing the conversion.

One automatic re-run, for the emulator and nothing else
-------------------------------------------------------

`android-ech` fails on infrastructure often enough to be a nuisance: the emulator comes up
half-dead, the APK install answers `Can't find service: package`, and the job finishes having run
zero tests. That is not a result about OkHttp, and re-running it by hand was the only response.
`rerun-flaky.yml` does it automatically now.

It is deliberately narrow, because an auto-retry is a way to hide real failures. Only
`android-ech` — the container and network suites fail for reasons worth reading, and extending
this to them would be claiming their failures are noise too. Only once, guarded on
`run_attempt == 1`: something that fails twice is either broken or flaky enough to fix rather than
absorb. And the retry is visible as attempt 2 of the same run, so the history shows both.

If it starts firing regularly, that is the signal to fix the emulator setup rather than to widen
the retry.

The resolver matrix
-------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,23 @@ class SvcParamTest {
).isEmpty()
}

/**
* A record offering only a protocol this client cannot speak.
*
* `alpn=h3` with no `h2` is the shape that would break connection setup if a client read the
* list as a requirement: OkHttp has no HTTP/3, and concluding the origin is unreachable would be
* wrong — `http/1.1` is implied into the set and is perfectly usable. The other half of the
* assertion is that `h2` is *not* invented to make the list more palatable, because a client
* that did would try a protocol the origin never offered.
*/
@Test
fun anAlpnListWithoutH2IsReadWithoutInventingIt() {
val alpn = metadataFor(H3_ONLY).alpnIds

assertThat(alpn, name = "$H3_ONLY ALPN ids").isNotNull().contains(Protocol.HTTP_1_1)
assertThat(alpn!!, name = "h2, which the record does not offer").doesNotContain(Protocol.HTTP_2)
}

private fun metadataFor(hostname: String): Dns.Record.ServiceMetadata {
val metadata = dns.records(hostname).filterIsInstance<Dns.Record.ServiceMetadata>()
assertThat(metadata, name = "$hostname HTTPS records").isNotEmpty()
Expand Down Expand Up @@ -211,6 +228,7 @@ class SvcParamTest {
const val NO_DEFAULT_ALPN = "nodefaultalpn.svcb.test"
const val MANDATORY = "mandatory.svcb.test"
const val UNKNOWN_PARAM = "unknownparam.svcb.test"
const val H3_ONLY = "h3only.svcb.test"

/**
* The name the resolver is reached by, and the one its certificate covers.
Expand Down
11 changes: 11 additions & 0 deletions ech-fixture/src/main/resources/ech-fixture/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,17 @@ func svcParamFixture(name string, targetPort int) ([]byte, bool) {
result = appendSvcParam(result, 3, port)
return result, true

// `alpn` naming a protocol the client cannot speak, and *not* naming one it can. A client with
// no HTTP/3 has to read this without concluding the origin is unreachable, and without
// inventing `h2` to make the list usable — issue #12's third bullet, which is a DNS question
// rather than a handshake one.
case "h3only.svcb.test":
result := appendUint16(nil, 1)
result = append(result, 0)
result = appendSvcParam(result, 1, []byte{2, 'h', '3'})
result = appendSvcParam(result, 3, port)
return result, true

// An unregistered parameter key alongside ordinary ones. The registry is designed to be
// extended, so a client must ignore what it does not recognise rather than reject the record
// — forward compatibility, tested the only way it can be.
Expand Down