elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.88k stars 587 forks source link

Add Plug.Conn.Status.valid?/1 #1156

Open halostatue opened 1 year ago

halostatue commented 1 year ago

I’m working on a library that accepts status codes (integer or atom) and want to verify that valid status values are used.

I can do this with:

try
  Plug.Conn.Status.code(value)
  true
rescue
  _ -> false
end

But this seems like it might be more generally useful if built into Plug.Status. Would a PR for this feature be accepted?

As well, it seems that Plug.Conn.Status.code/1 for the integer case isn’t entirely correct given the error raised with an unknown value in reason_phrase/1:

  def code(integer) when integer in 100..999 do
    integer
  end
…

That feels like it should be instead

```elixir
@valid_status_codes Map.keys(Map.merge(statuses, custom_statuses))
def code(integer) when integer in @valid_status_codes do
  integer
end
whatyouhide commented 1 year ago

The problem with HTTP status codes is that they are standardized, but not enforced. That means that a status code of 499 can be custom to an application using it, and still be perfectly valid HTTP. So, I’m not sure that valid?/1 here would make sense 🤔

halostatue commented 1 year ago

The name could be changed (known?/1, well_known?/1, defined?/1), but I was basing the name valid?/1 on the exceptions thrown for reason_phrase/1 and reason_atom/1 with unknown values and the fact that Plug.Conn.Status is compile-time extensible (which, before looking at this, I didn’t realize).

From my perspective, it's more important that something like valid?/1 be usable for the atom versions to reduce the need for runtime exception handling.