Skip to content

Commit 31d1b13

Browse files
mariduvmariduv
authored
[SCAN-81] Report ChunkUnit Panics (trufflesecurity#4367)
* Add common.RecoverWithHandler Allows panic recovery that still reports to Sentry, with caller-defined handling afterward, so there's no confusion about chained recover() or re-panicking. * Catch ChunkUnit panics and include them in report Keeps panics showing in Sentry, and includes panics in existing first-fatal-error behavior, but does not propagate panics further upward any longer. * Test SourceManager ChunkUnit panic reporting
1 parent 86d94c1 commit 31d1b13

File tree

3 files changed

+59
-10
lines changed

3 files changed

+59
-10
lines changed

‎pkg/common/recover.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,23 @@ func Recover(ctx context.Context) {
2727
}
2828
}
2929

30+
// RecoverWithHandler handles panics and reports to Sentry, then turns control
31+
// over to a provided function. This permits extra reporting in the same scope
32+
// without re-panicking, as recover() clears the state after it's called. Does
33+
// NOT block to flush sentry report.
34+
func RecoverWithHandler(ctx context.Context, callback func(error)) {
35+
if err := recover(); err != nil {
36+
panicStack := string(debug.Stack())
37+
if eventID := sentry.CurrentHub().Recover(err); eventID != nil {
38+
ctx.Logger().Info("panic captured", "event_id", *eventID)
39+
}
40+
ctx.Logger().Error(fmt.Errorf("panic"), panicStack,
41+
"recover", err,
42+
)
43+
callback(fmt.Errorf("panic: %e", err))
44+
}
45+
}
46+
3047
// RecoverWithExit handles panics and reports to Sentry before exiting.
3148
func RecoverWithExit(ctx context.Context) {
3249
if err := recover(); err != nil {

‎pkg/sources/source_manager.go

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,13 @@ func (s *SourceManager) EnumerateAndScan(ctx context.Context, sourceName string,
157157
ctx := context.WithValues(ctx,
158158
"source_manager_worker_id", common.RandomID(5),
159159
)
160-
defer common.Recover(ctx)
160+
defer common.RecoverWithHandler(ctx, func(err error) {
161+
progress.ReportError(Fatal{err})
162+
select {
163+
case s.firstErr <- err:
164+
default:
165+
}
166+
})
161167
defer cancel(nil)
162168
if err := s.run(ctx, source, progress, targets...); err != nil {
163169
select {
@@ -458,10 +464,13 @@ func (s *SourceManager) enumerateWithUnits(ctx context.Context, source SourceUni
458464

459465
// Produce units.
460466
func() {
461-
// TODO: Catch panics and add to report.
462467
report.StartEnumerating(time.Now())
463468
defer func() { report.EndEnumerating(time.Now()) }()
464469
ctx.Logger().V(2).Info("enumerating source with units")
470+
defer common.RecoverWithHandler(ctx, func(err error) {
471+
report.ReportError(Fatal{err})
472+
catchFirstFatal(Fatal{err})
473+
})
465474
if err := source.Enumerate(ctx, reporter); err != nil {
466475
report.ReportError(Fatal{err})
467476
catchFirstFatal(Fatal{err})
@@ -523,13 +532,17 @@ func (s *SourceManager) runWithUnits(ctx context.Context, source SourceUnitEnumC
523532
default:
524533
}
525534
}
535+
526536
// Produce units.
527537
go func() {
528-
// TODO: Catch panics and add to report.
529538
report.StartEnumerating(time.Now())
530539
defer func() { report.EndEnumerating(time.Now()) }()
531540
defer close(unitReporter.unitCh)
532541
ctx.Logger().V(2).Info("enumerating source")
542+
defer common.RecoverWithHandler(ctx, func(err error) {
543+
report.ReportError(Fatal{err})
544+
catchFirstFatal(Fatal{err})
545+
})
533546
if err := source.Enumerate(ctx, unitReporter); err != nil {
534547
report.ReportError(Fatal{err})
535548
catchFirstFatal(Fatal{err})
@@ -551,11 +564,14 @@ func (s *SourceManager) runWithUnits(ctx context.Context, source SourceUnitEnumC
551564
// Consume units and produce chunks.
552565
unitPool.Go(func() error {
553566
report.StartUnitChunking(unit, time.Now())
554-
// TODO: Catch panics and add to report.
555567
defer close(chunkReporter.chunkCh)
556568
id, kind := unit.SourceUnitID()
557569
ctx := context.WithValues(ctx, "unit_kind", kind, "unit", id)
558570
ctx.Logger().V(3).Info("chunking unit")
571+
defer common.RecoverWithHandler(ctx, func(err error) {
572+
report.ReportError(Fatal{ChunkError{Unit: unit, Err: err}})
573+
catchFirstFatal(Fatal{err})
574+
})
559575
if err := source.ChunkUnit(ctx, unit, chunkReporter); err != nil {
560576
report.ReportError(Fatal{ChunkError{Unit: unit, Err: err}})
561577
catchFirstFatal(Fatal{err})
@@ -597,11 +613,14 @@ func (s *SourceManager) scanWithUnit(ctx context.Context, source SourceUnitChunk
597613
var chunkErr error
598614
go func() {
599615
report.StartUnitChunking(unit, time.Now())
600-
// TODO: Catch panics and add to report.
601616
defer close(chunkReporter.chunkCh)
602617
id, kind := unit.SourceUnitID()
603618
ctx := context.WithValues(ctx, "unit_kind", kind, "unit", id)
604619
ctx.Logger().V(3).Info("chunking unit")
620+
defer common.RecoverWithHandler(ctx, func(err error) {
621+
report.ReportError(Fatal{ChunkError{Unit: unit, Err: err}})
622+
chunkErr = Fatal{err}
623+
})
605624
if err := source.ChunkUnit(ctx, unit, chunkReporter); err != nil {
606625
report.ReportError(Fatal{ChunkError{Unit: unit, Err: err}})
607626
chunkErr = Fatal{err}

‎pkg/sources/source_manager_test.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,11 @@ func (c *counterChunker) ChunkUnit(ctx context.Context, unit SourceUnit, reporte
8383
}
8484

8585
// Chunk method that always returns an error.
86-
type errorChunker struct{ error }
86+
type errorChunker struct{ cb func() error }
8787

88-
func (c errorChunker) Chunks(context.Context, chan *Chunk, ...ChunkingTarget) error { return c }
89-
func (c errorChunker) Enumerate(context.Context, UnitReporter) error { return c }
90-
func (c errorChunker) ChunkUnit(context.Context, SourceUnit, ChunkReporter) error { return c }
88+
func (c errorChunker) Chunks(context.Context, chan *Chunk, ...ChunkingTarget) error { return c.cb() }
89+
func (c errorChunker) Enumerate(context.Context, UnitReporter) error { return c.cb() }
90+
func (c errorChunker) ChunkUnit(context.Context, SourceUnit, ChunkReporter) error { return c.cb() }
9191

9292
// buildDummy is a helper function to enroll a DummySource with a SourceManager.
9393
func buildDummy(chunkMethod chunker) (Source, error) {
@@ -147,7 +147,18 @@ func TestSourceManagerWait(t *testing.T) {
147147

148148
func TestSourceManagerError(t *testing.T) {
149149
mgr := NewManager()
150-
source, err := buildDummy(errorChunker{fmt.Errorf("oops")})
150+
source, err := buildDummy(errorChunker{func() error { return fmt.Errorf("oops") }})
151+
assert.NoError(t, err)
152+
ref, err := mgr.EnumerateAndScan(context.Background(), "dummy", source)
153+
assert.NoError(t, err)
154+
<-ref.Done()
155+
assert.Error(t, ref.Snapshot().FatalError())
156+
assert.Error(t, mgr.Wait())
157+
}
158+
159+
func TestSourceManagerPanic(t *testing.T) {
160+
mgr := NewManager()
161+
source, err := buildDummy(errorChunker{func() error { panic("whoops!") }})
151162
assert.NoError(t, err)
152163
ref, err := mgr.EnumerateAndScan(context.Background(), "dummy", source)
153164
assert.NoError(t, err)
@@ -242,6 +253,7 @@ func (c *unitChunker) Chunks(ctx context.Context, ch chan *Chunk, _ ...ChunkingT
242253
}
243254
return nil
244255
}
256+
245257
func (c *unitChunker) Enumerate(ctx context.Context, rep UnitReporter) error {
246258
for _, step := range c.steps {
247259
if err := rep.UnitOk(ctx, CommonSourceUnit{ID: step.unit}); err != nil {
@@ -250,6 +262,7 @@ func (c *unitChunker) Enumerate(ctx context.Context, rep UnitReporter) error {
250262
}
251263
return nil
252264
}
265+
253266
func (c *unitChunker) ChunkUnit(ctx context.Context, unit SourceUnit, rep ChunkReporter) error {
254267
for _, step := range c.steps {
255268
id, _ := unit.SourceUnitID()

0 commit comments

Comments
 (0)