Interrupt backup from event listeners

[ffeytons]

This Pull Request is about giving the ability to interrupt the backup from the listeners of the following events:

• DumpingDatabase
• BackupManifestWasCreated
• BackupZipWasCreated

Those events can be used to execute last-minute processes e.g. files can be added to the backup with the BackupManifestWasCreated listener. These processes could generate errors, making the backup invalid.

Thanks to this PR, a BackupJobStepStatus object is added to those 3 events and it comes with the interruptBackupBecauseOfError method which will cause the interruption of the backup if called.

The previous situation was that any error occuring in one of the handlers of BackupManifestWasCreated or BackupZipWasCreated (when notifications were enabled for the latter) were ignored and the backup was continuing anyway, and was eventually marked as successful.
It is to be noted that exceptions thrown in the handler of DumpingDatabase would have caused the backup to be interrupted, but for the sake of coherence I've added the same interruption mechanism into it.

This PR is linked to this discussion: #1550

[freekmurze]

I think we could solve this problem more easily by introducing a HaltBackupException that we would rethrow when catched, effectively halting the backup process.

Could you make that change?

[ffeytons]

@freekmurze My initial idea was more or less similar. The problem is that events such as BackupManifestWasCreated and BackupZipWasCreated are considered as 'notifications' in the project: they are sent via the sendNotification method, which - by design - ignore any exception thrown in the handlers.

This PR was a way to workaround this design choice while minimizing the impact on the existing source code.

I think we have 3 options from here:

1. We 'detach' BackupManifestWasCreated and BackupZipWasCreated from the notification system (just like DumpingDatabase) and we allow just any exception to be thrown (and cause the backup interruption)
2. As you suggest, we let the BackupManifestWasCreated and BackupZipWasCreated be fired by the notification system and we focus on a single exception HaltBackupException as per your suggestion while ignoring all the other exceptions that would be thrown. This will require additional work for the BackupZipWasCreated event because its exceptions are not handled the same way depending on whether notifications are enabled or disabled (see method createZipContainingEveryFileInManifest of BackupJob.php).
3. We keep this PR as-is

What's your opinion?

[freekmurze]

Looking at the code, I think that it already works correctly. If you throw an exception in an event, you'll end up here, which is want you want to happen I think.

[spatie-bot]

Dear contributor,

because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it.