SRGSSR / pillarbox-apple

A next-generation reactive media playback ecosystem for Apple platforms.
https://testflight.apple.com/join/TS6ngLqf
MIT License
45 stars 6 forks source link

Fix error management #117

Closed defagos closed 1 year ago

defagos commented 1 year ago

As a user I want errors to be reported to me in the best possible way.

Context

The final 16.1 broke error management again. Now error messages ask to look at the error log, where probably the real error can be found.

Acceptance criteria

Tasks

Other non-related tasks found during development:

defagos commented 1 year ago

I guess we need to extract the error information from the errorLog() if available. Any other attempt I made was unsuccessful. Fortunately all information is available in the log and requires minimal parsing to be converted back to an error.

We should likely not subscribe to AVPlayerItemNewErrorLogEntryNotification, though, since we don't want to respond to errors which might be logged during playback (e.g. temporary network failure). For item errors we are only interested in looking at the error log when the item status changes to .failed. If no log entries are available we can fallback on the item error.

defagos commented 1 year ago

Best practices were discussed in a WWDC talk.

defagos commented 1 year ago

There is a bug with the error log when navigating back to a failed item in a queue (i.e. replacing the current item with an item which previously failed). The error log is cleared so that we cannot access detailed information (even if the item error property says to look at the error log).

I attempted to write a unit test but without success. The obvious solution is to cache the error when extracted the first time using an associated object.

To reproduce:

Sometimes (very often) the friendly error message is lost. If not repeat.

Note that the failing item should be skipped later (see todo list above) but the fix is still relevant IMHO (though it is a shame we cannot write a good unit test for it).

defagos commented 1 year ago

Test stream cleanup was really buggy. As a result the DVR stream would get messy if older chunks still existed. Two issues with the cleanup code:

I suspect this incorrect cleanup might explain the reliability issues we had on the CI server.

defagos commented 1 year ago

NSError wrapping is really required when passing errors to the resource loader, otherwise the result is unreliable (works in the simulator but not on device). Easy to check by removing wrapping from finishLoadingReliably(with:) implementation and checking the demo error examples.

defagos commented 1 year ago

Here is a way to have a .AVPlayerItemFailedToPlayToEndTime being emitted:

  1. Start playback for DVR content.
  2. Enable airplane mode during playback.
  3. Wait > 2 minutes (timeout).

We can therefore improve our item state management, though adding a unit test for this specific case is out of question.

defagos commented 1 year ago

~Depending on the language (currently the demo declares English as language) error messages returned by AVPlayer or our own messages are localized differently, e.g. in case of network errors. On a device in French we have:~

~Adding language files for French fixes this inconsistency (in this example files in en.lproj and fr.lproj). I will create another issue to add dummy language files to all packages and the demo since this is not in the scope of the present task.~

Update: I uninstalled the app and installed it again. The bundle might have been messed up.