This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new b8dd3c7 refactor(catalog/internal): Improve error handling in
WriteTableMetadata and WriteMetadata functions (#541)
b8dd3c7 is described below
commit b8dd3c7029f76a7185c4f307cb404fd4f49f5d7f
Author: Blue Li <[email protected]>
AuthorDate: Fri Aug 22 23:16:15 2025 +0800
refactor(catalog/internal): Improve error handling in WriteTableMetadata
and WriteMetadata functions (#541)
### Changes
- Updated the `WriteTableMetadata` and `WriteMetadata` functions to use
`errors.Join` for better error handling, ensuring that both JSON
encoding errors and file close errors are properly captured and
returned.
- This fix is particularly important for S3 storage where the actual
upload happens during `Close()`, and previous implementation would
silently ignore S3 upload failures.
---
catalog/glue/glue_test.go | 7 +++----
catalog/internal/utils.go | 13 ++++++++-----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/catalog/glue/glue_test.go b/catalog/glue/glue_test.go
index 19cce5e..89c72c4 100644
--- a/catalog/glue/glue_test.go
+++ b/catalog/glue/glue_test.go
@@ -1007,10 +1007,9 @@ func TestGlueCreateTableRollbackOnInvalidMetadata(t
*testing.T) {
catalog.WithLocation("s3://non-existent-test-bucket"))
// Should fail because LoadTable will fail to load the nonexistent
metadata
assert.Error(err)
- assert.Contains(err.Error(), "failed to create table")
- mockGlueSvc.AssertCalled(t, "CreateTable", mock.Anything,
mock.Anything, mock.Anything)
- mockGlueSvc.AssertCalled(t, "DeleteTable", mock.Anything,
mock.Anything, mock.Anything)
- mockGlueSvc.AssertCalled(t, "GetTable", mock.Anything, mock.Anything,
mock.Anything)
+ mockGlueSvc.AssertNotCalled(t, "CreateTable", mock.Anything,
mock.Anything, mock.Anything)
+ mockGlueSvc.AssertNotCalled(t, "DeleteTable", mock.Anything,
mock.Anything, mock.Anything)
+ mockGlueSvc.AssertNotCalled(t, "GetTable", mock.Anything,
mock.Anything, mock.Anything)
}
func TestRegisterTableMetadataNotFound(t *testing.T) {
diff --git a/catalog/internal/utils.go b/catalog/internal/utils.go
index 563756a..97ecdff 100644
--- a/catalog/internal/utils.go
+++ b/catalog/internal/utils.go
@@ -47,9 +47,11 @@ func WriteTableMetadata(metadata table.Metadata, fs
io.WriteFileIO, loc string)
if err != nil {
return nil
}
- defer out.Close()
- return json.NewEncoder(out).Encode(metadata)
+ return errors.Join(
+ json.NewEncoder(out).Encode(metadata),
+ out.Close(),
+ )
}
func WriteMetadata(ctx context.Context, metadata table.Metadata, loc string,
props iceberg.Properties) error {
@@ -68,9 +70,10 @@ func WriteMetadata(ctx context.Context, metadata
table.Metadata, loc string, pro
return nil
}
- defer out.Close()
-
- return json.NewEncoder(out).Encode(metadata)
+ return errors.Join(
+ json.NewEncoder(out).Encode(metadata),
+ out.Close(),
+ )
}
func UpdateTableMetadata(base table.Metadata, updates []table.Update,
metadataLoc string) (table.Metadata, error) {
