Project

General

Profile

Actions

Bug #7252

closed

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

Added by Tom Clegg over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
Keep
Target version:
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 1 (0 open1 closed)

Task #7787: Review branch: 7252-go-sdk-errorsResolvedRadhika Chippada09/10/2015Actions

Related issues

Related to Arvados - Idea #6260: [Keep] Integration test between data manager, API and keepstore.ResolvedRadhika Chippada08/19/2015Actions
Actions #1

Updated by Brett Smith over 8 years ago

  • Target version set to Arvados Future Sprints
Actions #2

Updated by Brett Smith over 8 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.

Actions #3

Updated by Brett Smith over 8 years ago

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

Updated by Radhika Chippada over 8 years ago

  • Story points changed from 0.5 to 1.0
Actions #5

Updated by Radhika Chippada over 8 years ago

  • Status changed from New to In Progress
Actions #6

Updated by Tom Clegg over 8 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)
       }
Actions #7

Updated by Radhika Chippada over 8 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.

Actions #8

Updated by Radhika Chippada over 8 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:128c2b5e228e1821384064ec50604a1463c29898.

Actions

Also available in: Atom PDF