fix(drivers): thirteen verified defects across the driver layer (#116–#128) - #130
Merged
Merged
Conversation
Closes #116, closes #117, closes #118, closes #119, closes #120, closes #121, closes #122, closes #123, closes #124, closes #125, closes #126, closes #127, closes #128. Attachment encoding is declared rather than guessed. `isBase64("test")` was true, so a four-byte text attachment went on the wire unencoded and the recipient's client decoded it into three bytes of noise — silently, on every driver. Guessing is not fixable here: "test" is valid as both text and base64. A string is now text unless the caller sets `encoding: "base64"`. A pool handoff that could not produce a connection dropped its waiter. The waiter was shifted off the queue and then `create()` was awaited; if that rejected, nothing resolved or rejected the `acquire()` promise and the send hung forever. Waiters carry both outcomes now. `await response.text()` sat outside the try in the shared fetch layer, and fetch resolves once the headers arrive. A socket reset while the body was streaming threw past the driver boundary and arrived as a non-retryable PROVIDER error — exactly backwards for a transient failure, so retry skipped it and the mail was lost. Resend's `sendBatch` dropped `idempotencyKey` entirely, so a retried batch duplicated every message in it. A batch now presents one key derived from its messages' keys: stable for the same batch, different for any other. It also sent attachments to an endpoint whose reference says it does not support them, which made behaviour depend on list length — `sendBatch([a])` attached the file, `sendBatch([a, b])` did not. Such a batch goes down the single-send path now. Neither batch driver chunked at the provider's cap (Resend 100, Postmark 500), so an over-large batch failed wholesale and that one error was reported against every message in it. A non-ASCII subject became one RFC 2047 encoded-word — measured at 124 characters against a cap of 75, and a 1997-octet header line against RFC 5322's hard limit of 998. `foldHeader` could not help, because base64 contains no spaces to fold on. Long values are split across several encoded-words now, never mid-character. An attachment filename went into two quoted MIME parameters unescaped, so `report.txt"; filename="payload.exe` arrived as two filenames. Quotes are escaped, and non-ASCII names use RFC 2231 rather than an encoded-word, which is not valid in a parameter. A driver now refuses what its own `features` say it cannot do. A template-only message reached SMTP with no body at all and was transmitted, accepted and reported `ok` — an entirely empty email. `scheduledAt` on a driver without scheduling simply went out immediately. `features` was declared by all six drivers and read by nothing. Only the offending message fails; the rest of the batch is unaffected, and a driver that declares no features is not second-guessed. SMTP announced `localhost.localdomain` on every connection: `resolveLocalName` reached `globalThis.require`, which does not exist in an ESM module, so the hostname branch was unreachable and the try/catch never even fired. It reads `node:os` properly now, once per driver, and keeps the fallback for a name that is not fully qualified. Postmark dropped the first tag's value — the one thing the comment above it claimed was not being dropped. Every tag now carries as metadata. DKIM documented PKCS1 support and only ever imported PKCS8, so the output of `openssl genrsa` failed with `Invalid keyData`. PKCS1 keys are wrapped in the PKCS8 envelope from RFC 5208; the resulting signature is byte-identical to the one the same key produces as PKCS8. `fallback` and `roundRobin` awaited `initialize()` on every send, bypassing the core's memoization because they reach their legs directly. 269 tests before, 293 now, with a regression test for each of the above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQAPDWLnDrdYHKhou3Ntvb
This was referenced Sep 2, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thirteen defects in the driver layer, each reproduced with a runnable probe before it was filed and each closed here with a regression test. Issues #116–#128.
Silent corruption and lost mail
#116 — a short text attachment arrived corrupt.
isBase64("test")wastrue, so a four-byte text attachment went on the wire unencoded and the recipient's client decoded it into"µë-". No error, on any driver.This one is not fixable by improving the guess:
"test"is valid text and valid base64, and so is"data","Name","MyReport". A string is now text unless the caller saysencoding: "base64".#117 — a failed pool handoff hung
send()forever.release()shifted the waiter off the queue and then awaitedcreate(). When that rejected — the provider stopped accepting connections, DNS went away — the waiter had already been removed and was never resolved or rejected.smtp.tsswallowed the rejection, so nothing was logged either. Waiters now carry both outcomes.#118 — a body dying mid-stream was classified as permanent.
await response.text()sat outside the try, andfetch()resolves as soon as the headers arrive. A proxy timing out while the body streamed threw past the driver boundary and arrived asPROVIDER/retryable: false— so retry skipped it and the mail was simply lost. It is a retryableNETWORKfailure now.#119 —
resend.sendBatchdroppedidempotencyKeyentirely.send()set the header;sendBatch()did not. Since any list of two or more goes down the batch path, a retried batch duplicated every message in it. A batch now presents one key derived from its messages' keys — stable for the same batch, different for any other.#124 — SMTP sent a completely empty email. A template-only message passes the core's body check, reaches SMTP with no
textand nohtml, andbuildMimerenders an empty part. It was transmitted, accepted, and reportedok.The fix is general:
driver.featureswas declared by all six drivers and read by nothing. The core reads it now, and a message asking for something the driver has said it cannot do comes backUNSUPPORTED. Only that message fails — the rest of the batch goes out — and a driver that declares no features at all is not second-guessed.Wrong on the wire
#121 — a long non-ASCII subject broke two RFCs at once. One encoded-word of 124 characters against RFC 2047's cap of 75, on a header line of 1997 octets against RFC 5322's hard limit of 998.
foldHeadercould not help: base64 has no spaces to fold on. Now split across several encoded-words, never mid-character, and round-tripping exactly.#122 — a quote in a filename opened a second parameter.
Quotes are escaped now, and a non-ASCII filename uses RFC 2231 (
filename*=UTF-8''…) rather than an encoded-word, which is not valid in a parameter.#120 —
resend.sendBatchsent attachments to an endpoint that rejects them. Resend's reference: "The attachments field is not supported yet." Behaviour therefore changed with list length —sendBatch([a])attached the invoice,sendBatch([a, b])did not. Such a batch takes the single-send path now.#123 — neither batch driver chunked at the provider's cap. 150 messages in one Resend request (cap 100), 600 in one Postmark request (cap 500). The provider rejected the request and that single error was reported against every message in it.
#125 — every SMTP connection announced
localhost.localdomain.resolveLocalName()reached forglobalThis.require, which does not exist in an ESM module, soreq?.(…)short-circuited, the hostname branch was unreachable, and thetry/catchnever even fired. Dead code that the docs described as working. It readsnode:osproperly now, once per driver, and keeps the fallback for a name that is not fully qualified.#126 — Postmark dropped the first tag's value. The comment above the code said "the rest carry as metadata so nothing the caller set is silently dropped";
tags[0].valuewas exactly what got silently dropped.#127 — DKIM documented PKCS1 and only imported PKCS8.
openssl genrsawrites PKCS1, and Web Crypto has no PKCS1 import format at all, so the most common key file failed with an unhelpfulInvalid keyData. PKCS1 keys are now wrapped in the RFC 5208 PrivateKeyInfo envelope — and the resulting signature is byte-identical to the one the same key produces as PKCS8, which is how I know the DER is right.#128 — the composites re-initialized on every send.
fallbackandroundRobinreach their legs directly, so the core's memoizedensureInitializednever saw them. Five sends, fiveinitialize()calls.Verification
269 tests before, 293 now. Lint, format, typecheck, JSR dry run, build with every
.d.mtspresent, bundle budgets, version consistency — all clean. Coverage 84.53% statements / 87.29% lines, above the floors set in #129.Note on versioning
Attachment.encodingis additive, but #116 changes what happens to a string attachment that looks like base64 and was not declared as such. That is a corruption fix rather than a feature removal, but it is a behaviour change — worth 1.1.0 rather than a patch, and worth a line in the release notes.Noticed, not fixed
Two leads in
connection.ts, the least-covered file left at 60%:sendInternalawaits"drain"with no error path. A socket erroring while backpressured could hang the write — the same shape as fix(drivers): a failed SMTP pool handoff drops the waiter, hanging send() forever #117. Not reproducible against the current fake server, so unproven rather than refuted.data/error/closelisteners on the pre-upgrade socket. Exercising it needs a TLS-capable fake server, which the harness does not have.Both are worth a follow-up, and both need the test harness extended before they can be settled either way.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GQAPDWLnDrdYHKhou3Ntvb