fix: reject HTTP error responses in InputFile downloads

[KnightNiwrem]

Note

The fix was written by Claude Fable 5.1 high on Claude Code CLI.
The PR body and fix verification were done by GPT-5.6 Sol xhigh on Codex CLI.

Summary

Reject unsuccessful responses in InputFile downloads using !response.ok, before their bodies can be uploaded as file contents. Only HTTP 200–299 responses pass this check.

Fixes #971.

This change:

• checks response.ok in the Deno, Node, and web URL helpers, and in Deno's existing Response input branch
• aborts unsuccessful URL downloads immediately
• rejects unsuccessful supplied Response objects without consuming or cancelling their bodies; callers remain responsible for consuming or cancelling those bodies
• throws a plain source Error containing the HTTP status through the existing lazy upload error path
• adds regression coverage for rejected 300/404/599 URL responses, upstream signal abortion, accepted 299 streams, and rejected 404 Response inputs

Why

fetch() resolves normally for HTTP error responses. Previously, a URL returning 404 or 503 could therefore produce a successfully delivered document containing the HTML error page, or a misleading Telegram media error such as IMAGE_PROCESS_FAILED.

Checking response.ok consistently requires a successful HTTP response across all three fetch implementations. URL downloads retain the existing redirect behavior and check the final response's ok value. A redirect ending at an HTTP 200 login page still passes this check.

Aborting a URL fetch started by grammY releases the connection without draining a potentially huge or endless unsuccessful response body. For a supplied Response, grammY rejects the unsuccessful status and leaves the body untouched. The caller decides whether to consume or cancel that body.

Successful responses retain streaming and the existing missing-body checks, including the failure for a bodyless 204 response.

Verification

The regression tests in test/types.test.ts cover the updated success criterion:

• URL responses with statuses 300, 404, and 599 reject with an error containing the HTTP status, and the signal passed to fetch() is aborted.
• A URL response with status 299 streams its bytes unchanged.
• A Deno Response with status 404 rejects with an error containing the HTTP status.

The current patch is at commit ce623338c0bdfba9acbbbe7e1bddbcf8d4f82c81; its source is available through pinned jsDelivr imports.

Earlier independent sandbox verification on Deno 2.9.6 exercised commit 5aa566824a4179a493bfb6177d700e4093d7d307, before the updates to response.ok and handling of supplied Response bodies:

• HTTP 4xx/5xx failures through URL, { url }, and Deno Response inputs rejected with HttpError.error containing the original plain status error. Multipart inspection confirmed zero file-body bytes were uploaded.
• Endless 404 URL downloads triggered server-side cancellation after one 1 KiB chunk.
• All 19 live Telegram checks passed: fifteen 404/503 error-source uploads aborted, and four valid HTTP 200 controls succeeded. Three control documents were downloaded and matched byte for byte and by SHA-256; the valid photo was delivered at 64 × 64.
• Sandbox formatting, type checks, and lint passed.

Those sandbox results apply to the earlier commit; the current patch has not been rerun in that sandbox. They do not establish cancellation of supplied Response bodies under the current patch, which leaves that responsibility to the caller. Runtime verification there covered Deno and the web InputFile implementation running in Deno, rather than actual Node or browser runtimes. Valid video/animation controls were not tested.

The fix preserves lazy multipart ordering: the Telegram fetch may start before an unsuccessful source is rejected. The existing HttpError wrapper also means auto-retry v2.0.2 retries these failures by default; the earlier sandbox run confirmed that rethrowHttpErrors: true propagates them immediately. Retry-policy changes are outside this fix.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.70%. Comparing base (b04bfee) to head (ce62333).
⚠️ Report is 145 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #972      +/-   ##
==========================================
- Coverage   45.52%   41.70%   -3.82%     
==========================================
  Files          19       19              
  Lines        5520     7296    +1776     
  Branches      360      466     +106     
==========================================
+ Hits         2513     3043     +530     
- Misses       3003     4168    +1165     
- Partials        4       85      +81     

☔ View full report in Codecov by Harness.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MobinAskari]

I tested out various URLs in different scenarios in my own project and it behaved exactly as I expected it to.

The fetch status check function implementation is kinda inconsistent. We have isErrorStatus in .deno.ts but we have a function in place when it comes to .node.ts and .web.ts. Not sure if it even matters since they do the same thing but I just wanted to point it out.

Also I used Node and installed it through npm+git, so I have no idea if it's behaving correctly on Deno, again it'll probably work just fine, I just wanted to notify.

This is more of a question for myself as I'm not really familiar with ins and outs of AbortController, so apologies for my ignorance, but isn't it used to cancel the request mid flight? if the request is already completed and we have the status, why are we calling abort at that point?

[KnightNiwrem]

I tested out various URLs in different scenarios in my own project and it behaved exactly as I expected it to.

The fetch status check function implementation is kinda inconsistent. We have isErrorStatus in .deno.ts but we have a function in place when it comes to .node.ts and .web.ts. Not sure if it even matters since they do the same thing but I just wanted to point it out.

Also I used Node and installed it through npm+git, so I have no idea if it's behaving correctly on Deno, again it'll probably work just fine, I just wanted to notify.

This is more of a question for myself as I'm not really familiar with ins and outs of AbortController, so apologies for my ignorance, but isn't it used to cancel the request mid flight? if the request is already completed and we have the status, why are we calling abort at that point?

When fetch has resolved with a Response, it does not mean that the connection has closed yet, or that the body bytes have all been downloaded to client. If that was the case, there would be no such thing as streaming - since all the body bytes would be in memory already.

Aborting a request, therefore, isn't just used to cancel a request mid-flight, but also to close the connection against the server, and signal that we don't want or care about the body bytes any more so it shouldn't bother sending any more, or tracking what is left to send.

[MobinAskari]

👍