Bug #7252
closed[SDKs] Return errors instead of calling log.Fatal in code that needs to be tested
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.
Updated by Brett Smith over 9 years ago
- Target version set to Arvados Future Sprints
Updated by Brett Smith about 9 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.
Updated by Brett Smith about 9 years ago
- Assigned To set to Radhika Chippada
- Target version changed from Arvados Future Sprints to 2015-12-02 sprint
Updated by Radhika Chippada about 9 years ago
- Story points changed from 0.5 to 1.0
Updated by Radhika Chippada about 9 years ago
- Status changed from New to In Progress
Updated by Tom Clegg about 9 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) }
Updated by Radhika Chippada about 9 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.
Updated by Radhika Chippada about 9 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:128c2b5e228e1821384064ec50604a1463c29898.