Skip to content

Commit

Permalink
Get rid of nolint:errcheck comments
Browse files Browse the repository at this point in the history 
This one had me second guessing myself quite a lot. Turns out defer
can't overwrite the return value so these are all not doing what it
looks like they're doing[1]. In order to influence the return'd error we
would have to used named return values so that err could be changed.

I was going to just do a simple defer'd recover like:

```
func() { _ = recover() }()
```

to keep catching the panics and get the same result as today but decided
to just do what was intendend instead.

1: https://go.dev/play/p/UIVjXdqajxI
  • Loading branch information
@mmlb
mmlb committed Apr 22, 2024
1 parent 20c1859 commit ed8ed1b
Showing 1 changed file with 30 additions and 60 deletions.
90 changes: 30 additions & 60 deletions actions/inventory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,14 +341,11 @@ func (a *InventoryCollectorAction) setDefaultAttributes() {

// CollectDrives executes drive collectors and merges the data into device.[]*Drive
// nolint:gocyclo // TODO(joel) if theres more conditionals to be added in here, the method is to be split up.
func (a *InventoryCollectorAction) CollectDrives(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectDrives(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.DriveCollectors == nil {
Expand Down Expand Up @@ -439,14 +436,11 @@ func (a *InventoryCollectorAction) findDriveByLogicalName(logicalName string, dr
// CollectDriveCapabilities executes drive capability collectors
//
// The capability collector is identified based on the drive logical name.
func (a *InventoryCollectorAction) CollectDriveCapabilities(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectDriveCapabilities(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

for _, drive := range a.device.Drives {
Expand Down Expand Up @@ -483,14 +477,11 @@ func (a *InventoryCollectorAction) CollectDriveCapabilities(ctx context.Context)
}

// CollectNICs executes nic collectors and merges the nic data into device.[]*NIC
func (a *InventoryCollectorAction) CollectNICs(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectNICs(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.NICCollector == nil {
Expand Down Expand Up @@ -532,14 +523,11 @@ func (a *InventoryCollectorAction) CollectNICs(ctx context.Context) error {
}

// CollectBMC executes the bmc collector and updates device bmc information
func (a *InventoryCollectorAction) CollectBMC(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectBMC(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.BMCCollector == nil {
Expand Down Expand Up @@ -569,14 +557,11 @@ func (a *InventoryCollectorAction) CollectBMC(ctx context.Context) error {
}

// CollectCPLDs executes the bmc collector and updates device cpld information
func (a *InventoryCollectorAction) CollectCPLDs(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectCPLDs(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.CPLDCollector == nil {
Expand Down Expand Up @@ -615,14 +600,11 @@ func (a *InventoryCollectorAction) CollectCPLDs(ctx context.Context) error {
}

// CollectBIOS executes the bios collector and updates device bios information
func (a *InventoryCollectorAction) CollectBIOS(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectBIOS(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.BIOSCollector == nil {
Expand Down Expand Up @@ -656,14 +638,11 @@ func (a *InventoryCollectorAction) CollectBIOS(ctx context.Context) error {
}

// CollectTPMs executes the TPM collector and updates device TPM information
func (a *InventoryCollectorAction) CollectTPMs(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectTPMs(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.TPMCollector == nil {
Expand Down Expand Up @@ -701,14 +680,11 @@ func (a *InventoryCollectorAction) CollectTPMs(ctx context.Context) error {
}

// CollectFirmwareChecksums executes the Firmware checksum collector and updates the component metadata.
func (a *InventoryCollectorAction) CollectFirmwareChecksums(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectFirmwareChecksums(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.FirmwareChecksumCollector == nil {
Expand Down Expand Up @@ -747,14 +723,11 @@ func (a *InventoryCollectorAction) CollectFirmwareChecksums(ctx context.Context)
}

// CollectUEFIVariables executes the UEFI variable collector and stores them on the device object
func (a *InventoryCollectorAction) CollectUEFIVariables(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectUEFIVariables(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if a.collectors.UEFIVarsCollector == nil {
Expand Down Expand Up @@ -792,14 +765,11 @@ func (a *InventoryCollectorAction) CollectUEFIVariables(ctx context.Context) err
}

// CollectStorageControllers executes the StorageControllers collectors and updates device storage controller data
func (a *InventoryCollectorAction) CollectStorageControllers(ctx context.Context) error {
// nolint:errcheck // deferred method catches a panic, error check not required.
defer func() error {
func (a *InventoryCollectorAction) CollectStorageControllers(ctx context.Context) (err error) {
defer func() {
if r := recover(); r != nil && a.failOnError {
return errors.Wrap(ErrPanic, string(debug.Stack()))
err = errors.Wrap(ErrPanic, string(debug.Stack()))
}

return nil
}()

if len(a.collectors.StorageControllerCollectors) == 0 {
Expand Down

0 comments on commit ed8ed1b

Please sign in to comment.