From 94752b4c1ef3f0184d12ae011bc9cb8df0e43696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean=20Fran=C3=A7ois=20CASSAN?= Date: Sat, 10 Feb 2024 12:26:37 +0100 Subject: [PATCH 1/2] chore(linter): improve error checkcing (#151) --- .golangci.yml | 1 + go.sum | 2 ++ immich/asset.go | 45 ++++++++++++++++++++++++------- immich/call.go | 56 ++++++++++++++++++--------------------- immich/call_test.go | 2 +- immich/e2e_asset_test.go | 4 +-- immich/metadata/direct.go | 10 +++++-- 7 files changed, 76 insertions(+), 44 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 37f8562d..e39e0789 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,3 +60,4 @@ linters: - unconvert - unparam - whitespace + - errcheck diff --git a/go.sum b/go.sum index 94775531..681190d9 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= +golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/immich/asset.go b/immich/asset.go index b1e9bff6..88e8b7b3 100644 --- a/immich/asset.go +++ b/immich/asset.go @@ -65,15 +65,42 @@ func (ic *ImmichClient) AssetUpload(ctx context.Context, la *browser.LocalAssetF la.Title = "No Name" + ext // fix #88, #128 } - m.WriteField("deviceAssetId", fmt.Sprintf("%s-%d", path.Base(la.Title), s.Size())) - m.WriteField("deviceId", ic.DeviceUUID) - m.WriteField("assetType", assetType) - m.WriteField("fileCreatedAt", la.DateTaken.Format(time.RFC3339)) - m.WriteField("fileModifiedAt", s.ModTime().Format(time.RFC3339)) - m.WriteField("isFavorite", myBool(la.Favorite).String()) - m.WriteField("fileExtension", ext) - m.WriteField("duration", formatDuration(0)) - m.WriteField("isReadOnly", "false") + err = m.WriteField("deviceAssetId", fmt.Sprintf("%s-%d", path.Base(la.Title), s.Size())) + if err != nil { + return + } + err = m.WriteField("deviceId", ic.DeviceUUID) + if err != nil { + return + } + err = m.WriteField("assetType", assetType) + if err != nil { + return + } + err = m.WriteField("fileCreatedAt", la.DateTaken.Format(time.RFC3339)) + if err != nil { + return + } + err = m.WriteField("fileModifiedAt", s.ModTime().Format(time.RFC3339)) + if err != nil { + return + } + err = m.WriteField("isFavorite", myBool(la.Favorite).String()) + if err != nil { + return + } + err = m.WriteField("fileExtension", ext) + if err != nil { + return + } + err = m.WriteField("duration", formatDuration(0)) + if err != nil { + return + } + err = m.WriteField("isReadOnly", "false") + if err != nil { + return + } // m.WriteField("isArchived", myBool(la.Archived).String()) // Not supported by the api h := textproto.MIMEHeader{} h.Set("Content-Disposition", diff --git a/immich/call.go b/immich/call.go index 7efba097..482e6ce1 100644 --- a/immich/call.go +++ b/immich/call.go @@ -93,7 +93,7 @@ func (ic *ImmichClient) newServerCall(ctx context.Context, api string, opts ...s } if sc.err == nil { for _, opt := range opts { - sc.joinError(opt(sc)) + _ = sc.joinError(opt(sc)) } } return sc @@ -171,12 +171,12 @@ func get(url string, opts ...serverRequestOption) requestFunction { } } -func post(url string, ctype string, opts ...serverRequestOption) requestFunction { +func post(url string, cType string, opts ...serverRequestOption) requestFunction { return func(sc *serverCall) *http.Request { if sc.err != nil { return nil } - return sc.request(http.MethodPost, sc.ic.endPoint+url, append(opts, setContentType(ctype))...) + return sc.request(http.MethodPost, sc.ic.endPoint+url, append(opts, setContentType(cType))...) } } @@ -234,13 +234,13 @@ func (sc *serverCall) _callDo(fnRequest requestFunction, opts ...serverResponseO req.URL.RawQuery = v.Encode() } if sc.ic.APITrace /* && req.Header.Get("Content-Type") == "application/json"*/ { - setTraceJSONRequest()(sc, req) + _ = sc.joinError(setTraceJSONRequest()(sc, req)) } resp, err = sc.ic.client.Do(req) // any non nil error must be returned if err != nil { - sc.joinError(err) + _ = sc.joinError(err) return sc.Err(req, nil, nil) } @@ -261,7 +261,7 @@ func (sc *serverCall) _callDo(fnRequest requestFunction, opts ...serverResponseO // We have a success for _, opt := range opts { - sc.joinError(opt(sc, resp)) + _ = sc.joinError(opt(sc, resp)) } if sc.err != nil { return sc.Err(req, resp, nil) @@ -299,18 +299,20 @@ func setJSONBody(object any) serverRequestOption { if sc.ic.APITrace { enc.SetIndent("", " ") } - if sc.joinError(enc.Encode(object)) == nil { - req.Body = io.NopCloser(b) + err := enc.Encode(object) + if err != nil { + return err } + req.Body = io.NopCloser(b) req.Header.Set("Content-Type", "application/json") - return sc.err + return err } } -func setContentType(ctype string) serverRequestOption { +func setContentType(cType string) serverRequestOption { return func(sc *serverCall, req *http.Request) error { - req.Header.Set("Content-Type", ctype) - return sc.err + req.Header.Set("Content-Type", cType) + return nil } } @@ -325,7 +327,7 @@ func setURLValues(values url.Values) serverRequestOption { } req.URL.RawQuery = rValues.Encode() } - return sc.err + return nil } } @@ -339,11 +341,8 @@ func responseJSON[T any](object *T) serverResponseOption { if resp.StatusCode == http.StatusNoContent { return nil } - - if sc.joinError(json.NewDecoder(resp.Body).Decode(object)) != nil { - return sc.err - } - return nil + err := json.NewDecoder(resp.Body).Decode(object) + return err } } return errors.New("can't decode nil response") @@ -362,8 +361,9 @@ func responseAccumulateJSON[T any](acc *[]T) serverResponseOption { return nil } arr := []T{} - if sc.joinError(json.NewDecoder(resp.Body).Decode(&arr)) != nil { - return sc.err + err := json.NewDecoder(resp.Body).Decode(&arr) + if err != nil { + return err } if len(arr) > 0 && sc.p != nil { sc.p.EOF = false @@ -390,17 +390,17 @@ func responseJSONWithFilter[T any](filter func(*T)) serverResponseOption { dec := json.NewDecoder(resp.Body) // read open bracket "[" _, err := dec.Token() - if sc.joinError(err) != nil { - return sc.err + if err != nil { + return nil } // while the array contains values for dec.More() { var o T // decode an array value (Message) - err := dec.Decode(&o) - if sc.joinError(err) != nil { - return sc.err + err = dec.Decode(&o) + if err != nil { + return err } if sc.p != nil { sc.p.EOF = false @@ -409,11 +409,7 @@ func responseJSONWithFilter[T any](filter func(*T)) serverResponseOption { } // read closing bracket "]" _, err = dec.Token() - if sc.joinError(err) != nil { - return sc.err - } - - return nil + return err } } return errors.New("can't decode nil response") diff --git a/immich/call_test.go b/immich/call_test.go index 1dd1b4af..6e4c206e 100644 --- a/immich/call_test.go +++ b/immich/call_test.go @@ -15,7 +15,7 @@ type testServer struct { func (ts *testServer) ServeHTTP(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(ts.responseStatus) - resp.Write([]byte(ts.responseBody)) + _, _ = resp.Write([]byte(ts.responseBody)) } func TestCall(t *testing.T) { diff --git a/immich/e2e_asset_test.go b/immich/e2e_asset_test.go index 47e1dfa9..3321422a 100644 --- a/immich/e2e_asset_test.go +++ b/immich/e2e_asset_test.go @@ -106,7 +106,7 @@ func checkImmich(t *testing.T, host, key, user string) { } writeFile(path.Join("DATA", "paginated.log"), paginated) - writeFile(path.Join("DATA", "allassets.log"), all) + writeFile(path.Join("DATA", "allAssets.log"), all) t.Logf("paginatedCounts: %+v", paginatedCounts) t.Logf("allCounts: %+v", allCounts) for _, u := range stat.UsageByUser { @@ -137,7 +137,7 @@ func TestAssetImmich(t *testing.T) { func (ic *ImmichClient) getAllAssetsIDs(ctx context.Context, opt *GetAssetOptions) ([]*Asset, error) { var r []*Asset - err := ic.newServerCall(ctx, "GetAllAssets").do(get("/asset", setUrlValues(opt.Values()), setAcceptJSON()), responseJSON(&r)) + err := ic.newServerCall(ctx, "GetAllAssets").do(get("/asset", setURLValues(opt.Values()), setAcceptJSON()), responseJSON(&r)) return r, err } diff --git a/immich/metadata/direct.go b/immich/metadata/direct.go index 220a7cf6..d82d00f6 100644 --- a/immich/metadata/direct.go +++ b/immich/metadata/direct.go @@ -69,7 +69,10 @@ func readHEIFDateTaken(r *sliceReader) (time.Time, error) { } filler := make([]byte, 6) - r.Read(filler) + _, err = r.Read(filler) + if err != nil { + return time.Time{}, err + } md, err := getExifFromReader(r) return md.DateTaken, err @@ -99,7 +102,10 @@ func readCR3DateTaken(r *sliceReader) (time.Time, error) { } filler := make([]byte, 4) - r.Read(filler) + _, err = r.Read(filler) + if err != nil { + return time.Time{}, err + } md, err := getExifFromReader(r) return md.DateTaken, err From 8dd556b04e5d72d320c31cd820b0660bb46b6d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean=20Fran=C3=A7ois=20CASSAN?= Date: Sat, 10 Feb 2024 12:29:16 +0100 Subject: [PATCH 2/2] Revert "chore(linter): improve error checkcing (#151)" (#152) This reverts commit 94752b4c1ef3f0184d12ae011bc9cb8df0e43696. --- .golangci.yml | 1 - go.sum | 2 -- immich/asset.go | 45 +++++++------------------------ immich/call.go | 56 +++++++++++++++++++++------------------ immich/call_test.go | 2 +- immich/e2e_asset_test.go | 4 +-- immich/metadata/direct.go | 10 ++----- 7 files changed, 44 insertions(+), 76 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e39e0789..37f8562d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,4 +60,3 @@ linters: - unconvert - unparam - whitespace - - errcheck diff --git a/go.sum b/go.sum index 681190d9..94775531 100644 --- a/go.sum +++ b/go.sum @@ -59,8 +59,6 @@ golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= -golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/immich/asset.go b/immich/asset.go index 88e8b7b3..b1e9bff6 100644 --- a/immich/asset.go +++ b/immich/asset.go @@ -65,42 +65,15 @@ func (ic *ImmichClient) AssetUpload(ctx context.Context, la *browser.LocalAssetF la.Title = "No Name" + ext // fix #88, #128 } - err = m.WriteField("deviceAssetId", fmt.Sprintf("%s-%d", path.Base(la.Title), s.Size())) - if err != nil { - return - } - err = m.WriteField("deviceId", ic.DeviceUUID) - if err != nil { - return - } - err = m.WriteField("assetType", assetType) - if err != nil { - return - } - err = m.WriteField("fileCreatedAt", la.DateTaken.Format(time.RFC3339)) - if err != nil { - return - } - err = m.WriteField("fileModifiedAt", s.ModTime().Format(time.RFC3339)) - if err != nil { - return - } - err = m.WriteField("isFavorite", myBool(la.Favorite).String()) - if err != nil { - return - } - err = m.WriteField("fileExtension", ext) - if err != nil { - return - } - err = m.WriteField("duration", formatDuration(0)) - if err != nil { - return - } - err = m.WriteField("isReadOnly", "false") - if err != nil { - return - } + m.WriteField("deviceAssetId", fmt.Sprintf("%s-%d", path.Base(la.Title), s.Size())) + m.WriteField("deviceId", ic.DeviceUUID) + m.WriteField("assetType", assetType) + m.WriteField("fileCreatedAt", la.DateTaken.Format(time.RFC3339)) + m.WriteField("fileModifiedAt", s.ModTime().Format(time.RFC3339)) + m.WriteField("isFavorite", myBool(la.Favorite).String()) + m.WriteField("fileExtension", ext) + m.WriteField("duration", formatDuration(0)) + m.WriteField("isReadOnly", "false") // m.WriteField("isArchived", myBool(la.Archived).String()) // Not supported by the api h := textproto.MIMEHeader{} h.Set("Content-Disposition", diff --git a/immich/call.go b/immich/call.go index 482e6ce1..7efba097 100644 --- a/immich/call.go +++ b/immich/call.go @@ -93,7 +93,7 @@ func (ic *ImmichClient) newServerCall(ctx context.Context, api string, opts ...s } if sc.err == nil { for _, opt := range opts { - _ = sc.joinError(opt(sc)) + sc.joinError(opt(sc)) } } return sc @@ -171,12 +171,12 @@ func get(url string, opts ...serverRequestOption) requestFunction { } } -func post(url string, cType string, opts ...serverRequestOption) requestFunction { +func post(url string, ctype string, opts ...serverRequestOption) requestFunction { return func(sc *serverCall) *http.Request { if sc.err != nil { return nil } - return sc.request(http.MethodPost, sc.ic.endPoint+url, append(opts, setContentType(cType))...) + return sc.request(http.MethodPost, sc.ic.endPoint+url, append(opts, setContentType(ctype))...) } } @@ -234,13 +234,13 @@ func (sc *serverCall) _callDo(fnRequest requestFunction, opts ...serverResponseO req.URL.RawQuery = v.Encode() } if sc.ic.APITrace /* && req.Header.Get("Content-Type") == "application/json"*/ { - _ = sc.joinError(setTraceJSONRequest()(sc, req)) + setTraceJSONRequest()(sc, req) } resp, err = sc.ic.client.Do(req) // any non nil error must be returned if err != nil { - _ = sc.joinError(err) + sc.joinError(err) return sc.Err(req, nil, nil) } @@ -261,7 +261,7 @@ func (sc *serverCall) _callDo(fnRequest requestFunction, opts ...serverResponseO // We have a success for _, opt := range opts { - _ = sc.joinError(opt(sc, resp)) + sc.joinError(opt(sc, resp)) } if sc.err != nil { return sc.Err(req, resp, nil) @@ -299,20 +299,18 @@ func setJSONBody(object any) serverRequestOption { if sc.ic.APITrace { enc.SetIndent("", " ") } - err := enc.Encode(object) - if err != nil { - return err + if sc.joinError(enc.Encode(object)) == nil { + req.Body = io.NopCloser(b) } - req.Body = io.NopCloser(b) req.Header.Set("Content-Type", "application/json") - return err + return sc.err } } -func setContentType(cType string) serverRequestOption { +func setContentType(ctype string) serverRequestOption { return func(sc *serverCall, req *http.Request) error { - req.Header.Set("Content-Type", cType) - return nil + req.Header.Set("Content-Type", ctype) + return sc.err } } @@ -327,7 +325,7 @@ func setURLValues(values url.Values) serverRequestOption { } req.URL.RawQuery = rValues.Encode() } - return nil + return sc.err } } @@ -341,8 +339,11 @@ func responseJSON[T any](object *T) serverResponseOption { if resp.StatusCode == http.StatusNoContent { return nil } - err := json.NewDecoder(resp.Body).Decode(object) - return err + + if sc.joinError(json.NewDecoder(resp.Body).Decode(object)) != nil { + return sc.err + } + return nil } } return errors.New("can't decode nil response") @@ -361,9 +362,8 @@ func responseAccumulateJSON[T any](acc *[]T) serverResponseOption { return nil } arr := []T{} - err := json.NewDecoder(resp.Body).Decode(&arr) - if err != nil { - return err + if sc.joinError(json.NewDecoder(resp.Body).Decode(&arr)) != nil { + return sc.err } if len(arr) > 0 && sc.p != nil { sc.p.EOF = false @@ -390,17 +390,17 @@ func responseJSONWithFilter[T any](filter func(*T)) serverResponseOption { dec := json.NewDecoder(resp.Body) // read open bracket "[" _, err := dec.Token() - if err != nil { - return nil + if sc.joinError(err) != nil { + return sc.err } // while the array contains values for dec.More() { var o T // decode an array value (Message) - err = dec.Decode(&o) - if err != nil { - return err + err := dec.Decode(&o) + if sc.joinError(err) != nil { + return sc.err } if sc.p != nil { sc.p.EOF = false @@ -409,7 +409,11 @@ func responseJSONWithFilter[T any](filter func(*T)) serverResponseOption { } // read closing bracket "]" _, err = dec.Token() - return err + if sc.joinError(err) != nil { + return sc.err + } + + return nil } } return errors.New("can't decode nil response") diff --git a/immich/call_test.go b/immich/call_test.go index 6e4c206e..1dd1b4af 100644 --- a/immich/call_test.go +++ b/immich/call_test.go @@ -15,7 +15,7 @@ type testServer struct { func (ts *testServer) ServeHTTP(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(ts.responseStatus) - _, _ = resp.Write([]byte(ts.responseBody)) + resp.Write([]byte(ts.responseBody)) } func TestCall(t *testing.T) { diff --git a/immich/e2e_asset_test.go b/immich/e2e_asset_test.go index 3321422a..47e1dfa9 100644 --- a/immich/e2e_asset_test.go +++ b/immich/e2e_asset_test.go @@ -106,7 +106,7 @@ func checkImmich(t *testing.T, host, key, user string) { } writeFile(path.Join("DATA", "paginated.log"), paginated) - writeFile(path.Join("DATA", "allAssets.log"), all) + writeFile(path.Join("DATA", "allassets.log"), all) t.Logf("paginatedCounts: %+v", paginatedCounts) t.Logf("allCounts: %+v", allCounts) for _, u := range stat.UsageByUser { @@ -137,7 +137,7 @@ func TestAssetImmich(t *testing.T) { func (ic *ImmichClient) getAllAssetsIDs(ctx context.Context, opt *GetAssetOptions) ([]*Asset, error) { var r []*Asset - err := ic.newServerCall(ctx, "GetAllAssets").do(get("/asset", setURLValues(opt.Values()), setAcceptJSON()), responseJSON(&r)) + err := ic.newServerCall(ctx, "GetAllAssets").do(get("/asset", setUrlValues(opt.Values()), setAcceptJSON()), responseJSON(&r)) return r, err } diff --git a/immich/metadata/direct.go b/immich/metadata/direct.go index d82d00f6..220a7cf6 100644 --- a/immich/metadata/direct.go +++ b/immich/metadata/direct.go @@ -69,10 +69,7 @@ func readHEIFDateTaken(r *sliceReader) (time.Time, error) { } filler := make([]byte, 6) - _, err = r.Read(filler) - if err != nil { - return time.Time{}, err - } + r.Read(filler) md, err := getExifFromReader(r) return md.DateTaken, err @@ -102,10 +99,7 @@ func readCR3DateTaken(r *sliceReader) (time.Time, error) { } filler := make([]byte, 4) - _, err = r.Read(filler) - if err != nil { - return time.Time{}, err - } + r.Read(filler) md, err := getExifFromReader(r) return md.DateTaken, err