Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled exception when LockCachingAudioSource receives an HTTP response without Content-Length header #570

Closed
agersant opened this issue Oct 26, 2021 · 9 comments
Assignees
Labels
3 testing bug Something isn't working

Comments

@agersant
Copy link
Contributor

Which API doesn't behave as documented, and how does it misbehave?
AudioPlayer.play() can result in an unhandled exception when attempting to play a LockCachingAudioSource - if the remote host serving audio does not respond with a content-length header.

Minimal reproduction project
https://github.com/agersant/just_audio/tree/just-audio-issue-570

To Reproduce (i.e. user steps, not code)

  1. Run the application from the just_audio_background/example directory
  2. Click the ! icon in the middle of the screen
  3. Observe crash

Error messages

Error message: Bad end position: -1
Call stack(s):

#0      _HttpOutgoing.addStream.<anonymous closure> (dart:_http/http_impl.dart:1810:9)
#1      _rootRunBinary (dart:async/zone.dart:1452:47)
#2      _CustomZone.runBinary (dart:async/zone.dart:1342:19)
#3      _FutureListener.handleError (dart:async/future_impl.dart:175:22)
#4      Future._propagateToListeners.handleError (dart:async/future_impl.dart:779:47)
#5      Future._propagateToListeners (dart:async/future_impl.dart:800:13)
#6      Future._completeError (dart:async/future_impl.dart:610:5)
#7      Future._asyncCompleteError.<anonymous closure> (dart:async/future_impl.dart:666:7)
#8      _rootRun (dart:async/zone.dart:1428:13)
#9      _CustomZone.run (dart:async/zone.dart:1328:19)
#10     _CustomZone.runGuarded (dart:async/zone.dart:1236:7)
#11     _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:1276:23)
#12     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
#13     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)
_proxyHandlerForSource.handler (c:\Users\agersant\Documents\GitHub\just_audio\just_audio\lib\just_audio.dart:2942)
<asynchronous gap> (Unknown Source:0)

Expected behavior
There are no errors and the audio source can be heard.

Screenshots
N/A

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: N/A

Smartphone (please complete the following information):

  • Device: Pixel 3a emulator
  • Android 11

Flutter SDK version

[√] Flutter (Channel stable, 2.0.5, on Microsoft Windows [Version 10.0.19042.1288], locale en-US)
[!] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    X Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/windows#android-setup for more details.
[√] Chrome - develop for the web
[√] Android Studio (version 4.1.0)
[√] VS Code (version 1.61.2)
[√] Connected device (3 available)

Additional context
Originally mentioned as a follow-up to #569

@ryanheise
Copy link
Owner

Sorry this one has been taking a bit longer to fix. LockCachingAudioSource only works if the original server supports range requests, and actually I thought this would be a safe assumption these days considering that Apple's OSs also expect this. But looking at the code, there is a way I can make this caching strategy work without range range requests, and I have been working on that.

Is the server under your control? At the moment, I'm getting server errors trying to access the URL you provided in your example so fixing efforts have stopped for the moment.

@agersant
Copy link
Contributor Author

agersant commented Oct 29, 2021

Sorry this one has been taking a bit longer to fix.

No worries, I am very grateful for just_audio and your continued efforts to improve it.

The server in the example repro is not under my control, but it seems to be working fine for me at the moment. The server I originally found the issue with is a Polaris server (I'm the maintainer of that), which does support range requests. From what I can tell in the HTTP spec, the presence of the content-length header also depends on the transfer-encoding header (see sections 14.3 and 4.4).

I'll try to find other servers in the wild that have the same behavior (although I haven't had much luck so far). Alternatively, you could run a Polaris server locally but I would understand if that was too big of a hassle.

@agersant
Copy link
Contributor Author

agersant commented Oct 29, 2021

I just found another example in the wild, using this URL: http://soundbible.com/grab.php?id=2219&type=mp3

@ryanheise
Copy link
Owner

Hi @agersant I've just pushed my work so far to address this on the fix/not_range_request branch.

I haven't actually tested whether this works on your example yet, but please feel free to try it and let me know how it goes. It's not something I want to rush into merging since I don't want to break the cases where range requests are typically supported.

@agersant
Copy link
Contributor Author

Thank you for the quick fix!

✔ I tested it on my original use-case (the Polaris server) and it worked great: played audio and did cache it as intended. The originSupportsRangeRequests check correctly picks up that the server supports range requests. As a side note, checking for the accept-range header might miss some servers that do support range requests. (see HTTP Spec 14.5). (although it's not an issue for my own use case)

✔ I also tested the https://filesamples.com/samples/audio/mp3/sample4.mp3 example and verified that audio played with no errors.

@ryanheise
Copy link
Owner

Glad to know it's working so far 👍

As a side note, checking for the accept-range header might miss some servers that do support range requests. (see HTTP Spec 14.5).

Thanks for pointing this out. Hopefully your comment will help either me or someone else to create a pull request in the future once someone runs into the issue.

@ryanheise
Copy link
Owner

This fix is now released in 0.9.17. I'll close this issue as resolved even though there is another edge case I haven't dealt with, but it will be easier to deal with it once someone runs into the issue and creates a new issue with the details.

@agersant
Copy link
Contributor Author

agersant commented Nov 9, 2021

Thank you!

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs, or use StackOverflow if you need help with just_audio.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 testing bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants