Skip to content

feat(http/unstable): HttpError#7132

Open
iuioiua wants to merge 5 commits into
denoland:mainfrom
iuioiua:http-error
Open

feat(http/unstable): HttpError#7132
iuioiua wants to merge 5 commits into
denoland:mainfrom
iuioiua:http-error

Conversation

@iuioiua
Copy link
Copy Markdown
Contributor

@iuioiua iuioiua commented May 8, 2026

This PR brings back the HttpError class, which I regret removing in #3736 (sorry Kitson 🥲). It's a very versatile class that can be called anywhere in the call stack and handled in a unified way in the "master" request handler. I've been using this in my own projects and love it.

This class is already implemented in Oak (similarly) and Fresh (a more minimal version).

I think it'd also be worth updating handlers within @std/http to throw errors instead of returning responses, allowing the dev to handle their own errors.

Written by me. Checked by Claude Code.

@github-actions github-actions Bot added the http label May 8, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.61%. Comparing base (a496da2) to head (b3c043e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7132      +/-   ##
==========================================
- Coverage   94.61%   94.61%   -0.01%     
==========================================
  Files         634      635       +1     
  Lines       51801    51845      +44     
  Branches     9329     9342      +13     
==========================================
+ Hits        49011    49052      +41     
- Misses       2216     2218       +2     
- Partials      574      575       +1     

☔ View full report in Codecov by Sentry.
📢 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.

@iuioiua iuioiua marked this pull request as ready for review May 8, 2026 00:58
@lionel-rowe
Copy link
Copy Markdown
Contributor

lionel-rowe commented May 11, 2026

What should happen here?

import { HttpError } from '@std/http/unstable-error'

const { addr: { hostname, port } } = Deno.serve(() => new Response(null, {
    status: 555,
}))

const res = await fetch(`http://${hostname}:${port}/`)
console.error(new HttpError(res.status))

Current behaviour:

  • Type checking fails upon construction ("Argument of type 'number' is not assignable to parameter of type 'ErrorStatus'.")
  • No runtime error
  • init.statusText is undefined

Also:

  • If res.status is something more normal like 404, type checking still fails as TypeScript can't infer it, but init.statusText is populated as expected.
  • If the status is a non-error status like 200 (whether statically analyzable or not), TypeScript rejects it but init.statusText is still populated.

IMO current behaviour is fine as you can just cast new HttpError(res.status as 400) or something (better yet: res.status as ErrorStatus, although that requires an additional import), it's just a little unexpected that the cast is necessary given that runtime behaviour isn't affected by the cast being incorrect.

@iuioiua
Copy link
Copy Markdown
Contributor Author

iuioiua commented May 12, 2026

On second thought, setting the statusText doesn't really provide any value. So I just removed it. Also, the error is intended to be thrown then caught within the request handler. And yes, if passing a number to the constructor, as ErrorStatus is the way to go.

Copy link
Copy Markdown

@fibibot fibibot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. JSDoc on the init property (lines 119-121) claims init.status and init.statusText always reflect the standard HTTP values for the given status code. Neither is true after the recent commit: init = { status, ...options?.init } never assigns statusText, and options.init.status wins over the constructor's status (spread comes last). Either re-add the auto-population, or rewrite the paragraph to describe the actual contract (init.status defaults to status and is overridable; init.statusText is whatever the user passes).
  • nit: while you're in there, the constructor doc block (lines 145-156) duplicates the class-level description verbatim. Constructor JSDoc can be a one-liner or just dropped — TypeDoc/LSP already shows the class doc on new HttpError(...).
  • nit: subtests are named "initialises with correct defaults" / "initialises with custom properties". The repo convention is symbolName() does X when Y — e.g. "new HttpError() defaults message to STATUS_TEXT[status]".

@iuioiua iuioiua requested a review from fibibot May 14, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants