HaveIBeenPwned / EmailAddressExtractor

A project to rapidly extract all email addresses from any files in a given path
BSD 3-Clause "New" or "Revised" License
64 stars 23 forks source link

Exceptions can disappear from view #59

Closed troyhunt closed 1 year ago

troyhunt commented 1 year ago

During the load process discussed in https://github.com/HaveIBeenPwned/EmailAddressExtractor/issues/58, an exception occurred. This looks like the same issue as already flagged in https://github.com/HaveIBeenPwned/EmailAddressExtractor/issues/54, the point of this issue is that had I not scrolled back through the console output, this would have been completely missed:

image

It's possible a file containing addresses was skipped so ideally, an unhandled exception in a thread should abort the entire process.

GStefanowich commented 1 year ago

When an exception occurs, it prompts a Do you want to continue? [yn], but if you use the --yes/-y flag it automatically continues

Seen here:

https://github.com/HaveIBeenPwned/EmailAddressExtractor/blob/8dced77b0e15acc49e5a21add7959633ceab3859/src/CommandLineProcessor.cs#L182-L211

Do you want it to always cancel, or use a different flag than the --yes maybe?

GStefanowich commented 1 year ago

Actually looks like this is just logged but not prompted

https://github.com/HaveIBeenPwned/EmailAddressExtractor/blob/8dced77b0e15acc49e5a21add7959633ceab3859/src/AddressExtractorMonitor.cs#L60-L64

I'll fix that to prompt properly

troyhunt commented 1 year ago

I suggest the -y flag should be broken into 2 separate flags:

  1. Supress prompt at the beginning of the run that confirms which files are within scope
  2. Supress prompts about continuing on error

Actually, I'm more inclined to just get rid of the second one altogether, is there a valid use case for supressing unhandled exceptions?

GStefanowich commented 1 year ago

I just figured if there's a breach that takes hours to process through, and an error occurs (eg; ~3 hours in) on one individual file it's better to keep the progress already made than to start the process all over again.

Then you could later just rerun the app against the individual file

troyhunt commented 1 year ago

That's reasonable, that one file throwing the exception could always be analysed in isolation later on. Let's just break it into a separate flag to resume on error.