File upload completion returns raw 500 (NoSuchUpload) instead of a clear error
Summary
POST /v1/files/{id}/uploaded can return an unhandled HTTP 500 instead of a structured client error.
Root cause
The request body's id field (FileUploadCompleted.id / S3FileUploadCompleted.id) is used directly as the S3 UploadId in complete_multipart_upload (polar/integrations/aws/s3/service.py). This is a different value from the File's own id used in the URL path: it must be the nested upload.id returned in the POST /v1/files/ response, not the top-level File id.
Neither S3FileUploadCompleted.id/.path nor S3FileUploadMultipart.id (in polar/integrations/aws/s3/schemas.py) have an OpenAPI Field(description=...) clarifying this distinction, so it isn't visible in Swagger UI or in the generated polar-python/polar-js SDKs (Speakeasy docs/examples just show a generic id: "<id>" placeholder). This makes it easy for an integrator to reuse the File's own id, which deterministically produces botocore.errorfactory.NoSuchUpload on every completion call.
Separately, S3Service.complete_multipart_upload has no try/except ClientError (unlike sibling methods get_object_or_raise/get_head_or_raise in the same file), so any S3-side rejection (NoSuchUpload, MalformedXML, InvalidPart, etc.) bubbles up as a raw, unhandled HTTP 500 instead of a structured 4xx PolarError.
Suggested fix
- Wrap the boto3 call in
S3Service.complete_multipart_uploadintry/except ClientError, converting it to a structured client-facing error (e.g.S3FileError), consistent with the other methods in that class. - Add
Field(description=...)toS3FileUploadCompleted.id/.pathandS3FileUploadMultipart.idexplicitly stating these must be theupload.id/upload.pathvalues from the file creation response, not the File's own id. Consider renaming toupload_idto remove the ambiguity outright. - Related:
S3FileUploadCompletedPart.checksum_sha256_base64lacks theStripValidatorthat was added tochecksum_etagin #13260 to fix a similar unhandled-ClientError-500 bug. The same whitespace-stripping should be applied there for consistency.
Repro
POST /v1/files/withservice: "product_media"→ 201, returns{ id: <file_uuid>, upload: { id: <s3_upload_id>, path, parts }, ... }.PUTthe file to the signed part URL → 200, valid ETag.POST /v1/files/{id}/uploadedwith body{ id: <file_uuid>, path, parts }(reusing the File's own id instead ofupload.id) → 500NoSuchUpload, every time.
Using upload.id (not the File's own id) in the body's id field is the correct/working call shape; regardless, the API should surface a clear 4xx error rather than a raw 500 when the ids don't match.
Sent by @allison-polar from File upload API 500 error investigation.
Source: polarsource/polar