treeverse / lakeFS

lakeFS - Data version control for your data lake | Git for data
https://docs.lakefs.io
Apache License 2.0
4.34k stars 342 forks source link

[Improvement]: S3 Gateway should respond with real error messages #5728

Open tshfc opened 1 year ago

tshfc commented 1 year ago

What happened?

When I call the CompleteMultiPartUpload API incorrectly, it always responds with a vague error message that doesn't help me, such as:

operation error S3: CompleteMultipartUpload, exceeded maximum number of attempts, 3, https response error StatusCode: 500, api error InternalError: We encountered an internal error, please try again.

But LakeFS actually logs the real error messages:

level=error msg="could not complete multipart upload" func="pkg/gateway/operations.(*PostObject).HandleCompleteMultipartUpload" file="build/pkg/gateway/operations/postobject.go:122" error="EntityTooSmall: Your proposed upload is smaller than the minimum allowed object size.\n\tstatus code: 400 method=POST operation_id=post_object service_name=s3_gateway upload_id=xxx 

I think the S3 Gateway API should also respond with a real error messag when encountered an error to help the caller.

lakeFS Version

0.94.1

arielshaqed commented 1 year ago

Ouch. This is obviously painful, sorry about that.

⚠️ Implementers please note!

In general it is NOT safe to pass errors messages from the block adaptor through to the external user: they might be (very) misleading, and possibly may even reveal some details that should not be visible to users. Even worse, error messages can change with versions of the underlying object storage.

However for multipart upload the basic error messages -- without any parameters from the actual call that we performed -- should be safe: These messages should all come from a similar operation on a block adaptor, rather than something like a KV. (We might get parameters from a non-S3 underlying store, of course -- but error messages will still be better than 500, api error InternalError: We encountered an internal error, please try again.)

github-actions[bot] commented 10 months ago

This issue is now marked as stale after 90 days of inactivity, and will be closed soon. To keep it, mark it with the "no stale" label.