Bug #7252

[SDKs] Return errors instead of calling log.Fatal in code that needs to be tested

Added by Tom Clegg almost 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
Keep
Target version:
Start date:
09/10/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Description

Currently, the Go SDK handles some runtime errors by calling log.Fatal(). In an SDK, this practice is unacceptable: the caller, not the library, should decide whether a given error should cause the entire process to exit abruptly. (The same goes for logs -- the application should be able to inspect and suppress logs if it wants to -- but while ugly, this is at least not fatal.)

The Go SDK should never exit -- via log.Fatal or anything else -- except at startup due to an error in the SDK's own code (e.g., it is OK to call regexp.MustCompile on a constant string). If it is possible for a function to encounter an error it can't handle, it should include an error in its return values. The caller must decide whether the error is fatal.


Subtasks

Task #7787: Review branch: 7252-go-sdk-errorsResolvedRadhika Chippada


Related issues

Related to Arvados - Story #6260: [Keep] Integration test between data manager, API and keepstore.Resolved08/19/2015

Associated revisions

Revision 128c2b5e
Added by Radhika Chippada over 3 years ago

closes #7252
Merge branch '7252-go-sdk-errors'

History

#1 Updated by Brett Smith almost 4 years ago

  • Target version set to Arvados Future Sprints

#2 Updated by Brett Smith almost 4 years ago

  • Subject changed from [Keep] [Data Manager] [SDKs] Return errors instead of calling log.Fatal in code that needs to be tested to [SDKs] Return errors instead of calling log.Fatal in code that needs to be tested
  • Description updated (diff)
  • Story points changed from 1.0 to 0.5

Split this into separate Go SDK and Data Manager stories. The latter is #7490.

#3 Updated by Brett Smith almost 4 years ago

  • Assigned To set to Radhika Chippada
  • Target version changed from Arvados Future Sprints to 2015-12-02 sprint

#4 Updated by Radhika Chippada almost 4 years ago

  • Story points changed from 0.5 to 1.0

#5 Updated by Radhika Chippada over 3 years ago

  • Status changed from New to In Progress

#6 Updated by Tom Clegg over 3 years ago

a5ed26a LGTM.

Nit: In test cases, t.Fatal() already displays the code and line number and so on, so I think it's more reasonable to just say

       if err != nil {
               t.Fatal(err)
       }

#7 Updated by Radhika Chippada over 3 years ago

Thanks. I merged as is without changing the t.Fatal usage during errors, but will keep it in mind for future tests and test updates.

#8 Updated by Radhika Chippada over 3 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:128c2b5e228e1821384064ec50604a1463c29898.

Also available in: Atom PDF