Excessive use of panic()

[vbanos]

We use panic() 63 times(!) From my experience its not good to terminate the whole program in so many places.
I would expect to use panic() on the key components to terminate the program if they receive some kind of errors from their sub-components.
Maybe this is an indication that we don't do error handling correctly.

Also, in unit tests I would expect to use t.Fatal instead of panic().

I will investigate more and get back with concrete improvement suggestions.

pkg/models/item_test.go
27:             panic(err)

pkg/models/url.go
110:            panic(err)

pkg/models/item.go
383:            panic("parent or child is nil")
419:                    panic(fmt.Sprintf("unable to close body, err: %s, seed id: %s", err.Error(), i.GetShortID()))

cmd/utils.go
48:             panic(fmt.Errorf("error starting pyroscope: %w", err))

internal/pkg/source/lq/consumer.go
101:                    panic(err)
145:                            panic("same seed received twice by lq.consumerSender")
178:                            panic(err)

internal/pkg/source/hq/consumer.go
104:                    panic(err)
110:                    panic(err)
158:                            panic("same seed received twice by hq.consumerSender")
191:                            panic(err)

internal/pkg/source/hq/seencheck.go
17:             panic(err)
49:             panic("no URLs to seencheck (can be caused if no fresh children were found)")

internal/pkg/preprocessor/preprocessor.go
90:             panic("preprocessor is not initialized")
94:             panic("seenchecker is already set")
133:                                    panic(fmt.Sprintf("seed consistency check failed with err: %s, seed id %s, worker_id %s", err.Error(), seed.GetShortID(), workerID))
137:                                    panic(fmt.Sprintf("preprocessor received seed with status %d, seed id: %s, worker_id %s", seed.GetStatus(), seed.GetShortID(), workerID))
164:            panic(err)
243:            panic(err)
263:            panic(err)

internal/pkg/preprocessor/seencheck/seencheck.go
38:             panic(err)
80:             panic(err)

internal/pkg/finisher/finisher.go
98:                                     panic("received nil seed")
102:                                    panic("received non-seed item")
108:                                    panic(fmt.Sprintf("seed consistency check failed with err: %s, seed id %s, worker id %s", err.Error(), seed.GetShortID(), workerID))
124:                                            panic(err)
133:                                    panic(err)

internal/pkg/reactor/reactor.go
99:             panic("item is not a seed")
136:                    panic("item is not a seed")
145:                    panic("item already present in reactor")

internal/pkg/utils/gzip.go
30:             panic(err)

internal/pkg/config/config.go
388:                    panic(err)

internal/pkg/postprocessor/item.go
51:                     panic(err)
68:     //              panic(err)
115:                                            panic(err)

internal/pkg/log/dumper/dumper.go
46:     panic(fmt.Sprintf("with item dump file: %s AND message: %s", dumpFilePath, message))

internal/pkg/postprocessor/postprocessor.go
96:                                     panic(fmt.Sprintf("seed consistency check failed with err: %s, seed id %s", err.Error(), seed.GetShortID()))
138:            panic(err)

internal/pkg/postprocessor/assets_test.go
30:             panic(err)

internal/pkg/postprocessor/extractor/css_embedded.go
98:                             panic(err)

internal/pkg/postprocessor/extractor/object_storage_test.go
16:             panic(err)

internal/pkg/postprocessor/extractor/base_test.go
14:             panic(err)

internal/pkg/postprocessor/extractor/html_test.go
24:             panic(err)
28:             panic(err)

internal/pkg/controler/channels.go
17:             panic("makeStageChannel: too many arguments, variadic argument should be omitted or a single integer")

internal/pkg/controler/pipeline.go
51:             panic(err)
61:             panic(err)
77:                     panic(err)
86:             panic(err)
94:                     panic(err)
102:            panic(err)
109:            panic(err)
119:            panic(err)
141:            panic(err)
147:            panic(err)
155:                            panic(err)
164:                            panic(err)

internal/pkg/controler/watchers/disk_unix.go
15:             panic(fmt.Sprintf("Error retrieving disk stats: %v\n", err))

internal/pkg/controler/watchers/disk_windows.go
17:             panic(fmt.Sprintf("Error retrieving disk stats: %v\n", err))

internal/pkg/archiver/worker.go
175:                                    panic(fmt.Sprintf("seed consistency check failed with err: %s, seed id %s", err.Error(), seed.GetShortID()))
209:            panic(err)

internal/pkg/archiver/headless/archiver.go
421:                    panic(closeErr)

internal/pkg/archiver/general/archiver.go
48:             panic("request is nil")

internal/pkg/archiver/general/body.go
66:                             panic(closeErr)
75:                             panic(closeErr)

internal/pkg/archiver/headless/browser.go
96:             panic(fmt.Sprintf("Unsupported DevTools-Protocol version: %s, expected 1.3", HeadlessBrowser.MustVersion().ProtocolVersion))

e2e/log/log.go
28:                                     panic(fmt.Sprintf("duplicate key %s in record", d.Key()))
39:                     panic(d.Err())

[vbanos]

We also use os.Exit in many places, these must be replaced by error handling / panic.

main.go
21:             os.Exit(1)

internal/pkg/config/config.go
225:                            os.Exit(1)
361:            os.Exit(1)

internal/pkg/postprocessor/extractor/main_test.go
15:     os.Exit(m.Run())

internal/pkg/controler/pipeline.go
46:             os.Exit(1)

internal/pkg/controler/signal.go
32:                     os.Exit(1)

internal/pkg/archiver/headless/browser.go
72:                     os.Exit(1)

internal/pkg/archiver/warc.go
72:                     os.Exit(1)
87:                     os.Exit(1)

[equals215]

Completely agree for the os.Exit()

panic() were used with parsimony, most of them are absolutely critical to the program and shouldn't be removed ; panics are there to tell the developer they've might made an horrible mistake that could ruin their life, and ours.