supabase / storage

S3 compatible object storage service that stores metadata in Postgres
https://supabase.com/docs/guides/storage
Apache License 2.0
821 stars 117 forks source link

HEAD /object/authenticated/<bucket>/not_found should use status code 404 #323

Open Xuanwo opened 1 year ago

Xuanwo commented 1 year ago

Bug report

Describe the bug

supabase storage will return 400 error code for HEAD request while object not found.

To Reproduce

https://github.com/supabase/storage-api/blob/1794d04ab0e7259d9058293e257f442015252ec3/src/storage/renderer/renderer.ts#L45-L52

Expected behavior

The status code should be 404 since HEAD request will not have a body and there is no way for client to know what happened.

Screenshots

None

System information

Not related

Additional context

https://github.com/apache/incubator-opendal/pull/2200

xyjixyjixyji commented 1 year ago

Actually the metadata like the status code 404 should be returned in the header for most requests instead of in the body. Currently I found that supabase always returns 400 in the header and stores its actual status_code in the body.

I am curious about the reason under the hood :)

fenos commented 1 year ago

Returning 400 instead of 404 was a legacy decision to force the user to branch error handling with the response code. It is arguably not semantically correct as you mentioned.

We are happy to rethink our error handling status code for the next version and return the correct status code

Xuanwo commented 1 year ago

We are happy to rethink our error handling status code for the next version and return the correct status code

Thanks! The most confusing thing here is HEAD can't return a body so users don't need what error happened.

rmkerr commented 1 month ago

Hey! Just want to jump in here and say this would be nice for us at talc. Making this change would improve the clarity of our opentelemetry logging, as we categorize some logs into errors based on status code.