How to get PUT request status code when using usePresignedUpload?

[williamrjribeiro]

Hello there. 👋

I'm dealing with a very simple scenario that I'm unable to overcome.

Our app uploads files using pre-signed URLs and we encountered a bug where all PUT requests to the bucket were resulting in 403 due to some configuration error. Our UI was not dealing with this kind of error and was wrongly showing "upload success" messages.

Now that we have identified the root cause of the problem, I'm not able to figure out how to get the status code of the PUT request performed by usePresignedUpload.

From what I've seen in the code, one of the easiest solutions would be to pass the xhr reference to resolve and reject calls:
https://github.com/ryanto/next-s3-upload/blob/master/packages/next-s3-upload/src/hooks/use-presigned-upload.ts#L21

But please let me know if I overlooked something.
Cheers!

[ryanto]

Hey there! Thanks for the issue!

Quick question: Do you need the status code, or is knowing if the upload was successful or not enough?

[williamrjribeiro]

Well, I was hoping that the app code would determine if the upload was successful or not by using the xhr object as it contains the status code and more info about the request.

But yeah, just knowing somehow that the upload was successful would be enough.

Maybe this info could be obtained from each file from files of const { uploadToS3, files } = usePresignedUpload();. 💭

[williamrjribeiro]

Wait a sec... I think there's a small bug here: https://github.com/ryanto/next-s3-upload/blob/master/packages/next-s3-upload/src/hooks/use-presigned-upload.ts#L20

When xhr.status === 500 //true, the if statement resolves to true. Or is that intended?

This is how I suppose it should work:

xhr.onload = () => {
  if (xhr.status >= 200 && xhr.status < 300) { // Lib considers 4xx & 5xx as errors
    resolve(xhr);
  } else {
    reject(xhr);
  }
};

xhr.onerror = () => reject(xhr.statusText);

I know this is unrelated to my original question but it was bugging me. 😅 If you preffer, I can create a new issue.

[ryanto]

Ha nice catch! Absolutely a bug.

No need for new issue, but if you want to PR a fix go for it!

[williamrjribeiro]

Oh, cool! Let me give it a shot.

Do you mind if in the PR I include the bug fix and my suggestion for provingding the xhr object? 😬

[ryanto]

I think we should skip the xhr object for now, just because I'm not sure I want to expose the xhr object forever. In the future we might switch this to fetch, and that would mean the value would change since fetch's response is a different object.

If the promise is rejecting correctly then you'll know if an upload worked:

try {
  await uploadToS3(...);
} catch(e) {
  console.error("upload failed");
}

[ryanto]

Hey @williamrjribeiro thanks again for spotting that bug and getting it fixed in the PR! I really appreciate it.

I think for the status code I'd rather not expose it at this point. There's a few reasons, but in no particular order:

• There's multiple requests that happen when uploading, and I'm worried about tying the promise to the error from one of the requests (and not the others)
• It causes the usePresignedUpload to drift from the useS3Upload API. I'd like to keep these as similar as possible
• Long term I want to make it as easy as possible to maintain and upgrade this library. If we start adding APIs that aren't found in the AWS sdk it makes that harder.

I'm always open to having my mind changed, so if you have use cases that are only solved by knowing the status please let me know.

Also thanks again for that bugfix! Super happy we got one that one solved 😄