Skip to content

Commit 9ddcab7

Browse files
committed
(catalog,manifest): handle some ignored errors
Also enable errcheck linter for future. Ignore table printer errors for cli which are self observable and mostly fine. Ignoring it in tests. There are many violations, could be enabled separately if needed. Signed-off-by: ferhat elmas <[email protected]>
1 parent b81fbd8 commit 9ddcab7

File tree

4 files changed

+27
-7
lines changed

4 files changed

+27
-7
lines changed

.golangci.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,26 @@ version: "2"
1919
linters:
2020
default: none
2121
enable:
22+
- errcheck
2223
- misspell
2324
- nlreturn
2425
- perfsprint
26+
settings:
27+
errcheck:
28+
exclude-functions:
29+
- (github.com/pterm/pterm.TablePrinter).Render
30+
- (github.com/pterm/pterm.TreePrinter).Render
2531
exclusions:
2632
generated: lax
2733
presets:
2834
- comments
2935
- common-false-positives
3036
- legacy
3137
- std-error-handling
38+
rules:
39+
- linters:
40+
- errcheck
41+
path: _test\.go$
3242
paths:
3343
- third_party$
3444
- builtin$

catalog/rest/rest.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,15 @@ func doPostAllowNoContent[Payload, Result any](ctx context.Context, baseURI *url
357357
func handleNon200(rsp *http.Response, override map[int]error) error {
358358
var e errorResponse
359359

360-
dec := json.NewDecoder(rsp.Body)
361-
dec.Decode(&struct {
362-
Error *errorResponse `json:"error"`
363-
}{Error: &e})
360+
// Only try to decode if there's a body (HEAD requests don't have one)
361+
if rsp.ContentLength != 0 {
362+
decErr := json.NewDecoder(rsp.Body).Decode(&struct {
363+
Error *errorResponse `json:"error"`
364+
}{Error: &e})
365+
if decErr != nil && decErr != io.EOF {
366+
return fmt.Errorf("%w: failed to decode error response: %s", ErrRESTError, decErr.Error())
367+
}
368+
}
364369

365370
if override != nil {
366371
if err, ok := override[rsp.StatusCode]; ok {
@@ -569,7 +574,10 @@ func (r *Catalog) fetchAccessToken(cl *http.Client, creds string, opts *options)
569574

570575
switch rsp.StatusCode {
571576
case http.StatusUnauthorized, http.StatusBadRequest:
572-
defer rsp.Request.GetBody()
577+
defer func() {
578+
_, _ = io.Copy(io.Discard, rsp.Body)
579+
_ = rsp.Body.Close()
580+
}()
573581
dec := json.NewDecoder(rsp.Body)
574582
var oauthErr oauthErrorResponse
575583
if err := dec.Decode(&oauthErr); err != nil {

catalog/sql/sql.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ type sqlIcebergNamespaceProps struct {
154154
}
155155

156156
func withReadTx[R any](ctx context.Context, db *bun.DB, fn func(context.Context, bun.Tx) (R, error)) (result R, err error) {
157-
db.RunInTx(ctx, &sql.TxOptions{ReadOnly: true}, func(ctx context.Context, tx bun.Tx) error {
157+
err = db.RunInTx(ctx, &sql.TxOptions{ReadOnly: true}, func(ctx context.Context, tx bun.Tx) error {
158158
result, err = fn(ctx, tx)
159159

160160
return err

manifest.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,9 @@ func constructPartitionSummaries(spec PartitionSpec, schema *Schema, partitions
10261026

10271027
for _, part := range partitions {
10281028
for i, field := range partType.FieldList {
1029-
fieldStats[i].update(part[field.ID])
1029+
if err := fieldStats[i].update(part[field.ID]); err != nil {
1030+
return nil, fmt.Errorf("error updating field stats for partition %d: %s: %s", i, field.Name, err)
1031+
}
10301032
}
10311033
}
10321034

0 commit comments

Comments
 (0)