Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve invalid_mime_type error to report the attempted mime type #403

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

seanedwards
Copy link
Contributor

@seanedwards seanedwards commented Dec 12, 2023

What kind of change does this PR introduce?

Improve error message clarity

What is the current behavior?

When uploading an object to supabase storage, the mime type is checked against the allowed mime types on the bucket. If the mime type is not allowed, an error message is reported, but the message does not say what the mime type was.

Before:

mime type not supported

What is the new behavior?

If the mime type is not allowed, the message informs the user what mime type was attempted:

mime type ${mimeType} is not supported

Additional context

I got stuck on a really hairy problem today that took quite a bit of digging to figure out. Here's what happened:

  • I used fetch() to retrieve a photo from a URL. This resulted in a Blob type.
const blob: Blob = await fetch(url).then(res => res.blob())
  • I used upload() to put that image into object storage. (Note: I already had the contentType variable from another source, so I expected that this file would be uploaded as image/jpeg)
await supabase.storage.from(myBucket).upload(filename, blob, { contentType })
  • However, for some reason, the server from which I was retrieving the photo was returning Content-type: application/octet-stream. See this community post on Airtable

  • The Blob data type already includes a mime type, and the Supabase storage API uses this instead of the explicitly declared contentType parameter! This means that, even though I was explicitly specifying the content type, I was inadvertently uploading my file with the application/octet-stream mime-type, which my bucket did not allow!

  • There were no logs or error messages to indicate that I was attempting to upload an application/octet-stream content type. I checked the Supabase storage logs, and there was no error message indicating a failed upload, and the context was not in the error message. With the content type explicitly specified as image/jpeg, and with no other clues that the upload was actually going out as application/octet-stream, I was completely stumped.

  • I was finally able to get to the bottom of this based on this clue from the supabase docs. This line made me try leaving out the contentType parameter, and then that finally got me to log the blob.type field. Once I converted the blob to a stream and specified the correct content type, the upload worked.
    image

It took me many hours to get to the bottom of why this was happening. For this pull request, I opted not to try to make the contentType parameter take precedence over the Blob's content type. I think it would make more sense to use the explicitly declared contentType field, but I didn't want to change behavior in case there was a good reason why it works this way. However, I did try to make the resulting error message a little more explicit. If the error had told me what the attempted mime type was, I think I would have realized what was happening a lot sooner.

I also updated the mime type is not formatted properly message, and added a test for it.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7177003355

  • 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.146%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/storage/uploader.ts 1 2 50.0%
Totals Coverage Status
Change from base Build 7031108426: 0.0%
Covered Lines: 7564
Relevant Lines: 8918

💛 - Coveralls

@fenos fenos merged commit b8ac8f6 into supabase:master Dec 27, 2023
1 check passed
@fenos
Copy link
Contributor

fenos commented Dec 27, 2023

Thank you!

Copy link

github-actions bot commented Jan 2, 2024

🎉 This PR is included in version 0.43.13 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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