Skip to content

Commit

Permalink
fix: Improve Error handling for Unsupported Sources (#1625)
Browse files Browse the repository at this point in the history
# Description

- feat: Improved error description for unsupported file formats
- fix: Handle white space and special characters in URL and Assets (Web
& Darwin)
- test: Test files without file extension (not playable Darwin)

## Related Issues

Closes #1494
Closes #748
Closes #972
Closes #1546

---------

Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
  • Loading branch information
Gustl22 and spydon authored Sep 29, 2023
1 parent 8c3a213 commit a4d8442
Show file tree
Hide file tree
Showing 19 changed files with 310 additions and 87 deletions.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ final wavUrl1TestData = LibSourceTestData(
duration: const Duration(milliseconds: 451),
);

final specialCharUrlTestData = LibSourceTestData(
source: UrlSource(wavUrl3),
duration: const Duration(milliseconds: 451),
);

final mp3Url1TestData = LibSourceTestData(
source: UrlSource(mp3Url1),
duration: const Duration(minutes: 3, seconds: 30, milliseconds: 77),
Expand All @@ -57,6 +62,16 @@ final invalidAssetTestData = LibSourceTestData(
duration: null,
);

final specialCharAssetTestData = LibSourceTestData(
source: AssetSource(specialCharAsset),
duration: const Duration(milliseconds: 451),
);

final noExtensionAssetTestData = LibSourceTestData(
source: AssetSource(noExtensionAsset),
duration: const Duration(milliseconds: 451),
);

final nonExistentUrlTestData = LibSourceTestData(
source: UrlSource('non_existent.txt'),
duration: null,
Expand Down
72 changes: 72 additions & 0 deletions packages/audioplayers/example/integration_test/lib_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:audioplayers/audioplayers.dart';
import 'package:audioplayers_example/tabs/sources.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
Expand All @@ -12,8 +13,79 @@ void main() async {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
final features = PlatformFeatures.instance();
final isAndroid = !kIsWeb && defaultTargetPlatform == TargetPlatform.android;
final isIOS = !kIsWeb && defaultTargetPlatform == TargetPlatform.iOS;
final isMacOS = !kIsWeb && defaultTargetPlatform == TargetPlatform.macOS;
final audioTestDataList = await getAudioTestDataList();

testWidgets('test asset source with special char',
(WidgetTester tester) async {
final player = AudioPlayer();

await tester.pumpLinux();
await player.play(specialCharAssetTestData.source);
await tester.pumpAndSettle();
// Sources take some time to get initialized
await tester.pump(const Duration(seconds: 8));
await player.stop();

await tester.pumpLinux();
await player.dispose();
});

testWidgets(
'test device file source with special char',
(WidgetTester tester) async {
final player = AudioPlayer();

await tester.pumpLinux();
final path = await player.audioCache.loadPath(specialCharAsset);
expect(path, isNot(contains('%'))); // Ensure path is not URL encoded
await player.play(DeviceFileSource(path));
await tester.pumpAndSettle();
// Sources take some time to get initialized
await tester.pump(const Duration(seconds: 8));
await player.stop();

await tester.pumpLinux();
await player.dispose();
},
skip: kIsWeb,
);

testWidgets('test url source with special char', (WidgetTester tester) async {
final player = AudioPlayer();

await tester.pumpLinux();
await player.play(specialCharUrlTestData.source);
await tester.pumpAndSettle();
// Sources take some time to get initialized
await tester.pump(const Duration(seconds: 8));
await player.stop();

await tester.pumpLinux();
await player.dispose();
});

testWidgets(
'test url source with no extension',
(WidgetTester tester) async {
final player = AudioPlayer();

await tester.pumpLinux();
await player.play(noExtensionAssetTestData.source);
await tester.pumpAndSettle();
// Sources take some time to get initialized
await tester.pump(const Duration(seconds: 8));
await player.stop();

await tester.pumpLinux();
await player.dispose();
},
// Darwin does not support files without extension unless its specified
// #803, https://stackoverflow.com/a/54087143/5164462
skip: isIOS || isMacOS,
);

group('play multiple sources', () {
testWidgets(
'play multiple sources simultaneously',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ void main() async {
testData: invalidAssetTestData,
);
fail('PlatformException not thrown');
// ignore: avoid_catches_without_on_clauses
} catch (e) {
expect(e, isInstanceOf<PlatformException>());
} on PlatformException catch (e) {
expect(e.message, startsWith('Failed to set source.'));
}
await tester.pumpLinux();
},
Expand All @@ -64,8 +63,8 @@ void main() async {
);
fail('PlatformException not thrown');
// ignore: avoid_catches_without_on_clauses
} catch (e) {
expect(e, isInstanceOf<PlatformException>());
} on PlatformException catch (e) {
expect(e.message, startsWith('Failed to set source.'));
}
await tester.pumpLinux();
},
Expand Down Expand Up @@ -539,8 +538,8 @@ extension on WidgetTester {
if (source is UrlSource) {
await platform.setSourceUrl(playerId, source.url);
} else if (source is AssetSource) {
final url = await AudioCache.instance.load(source.path);
await platform.setSourceUrl(playerId, url.path, isLocal: true);
final cachePath = await AudioCache.instance.loadPath(source.path);
await platform.setSourceUrl(playerId, cachePath, isLocal: true);
} else if (source is BytesSource) {
await platform.setSourceBytes(playerId, source.bytes);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/audioplayers/example/lib/tabs/sources.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ final host = useLocalServer ? 'http://$localhost:8080' : 'https://luan.xyz';

final wavUrl1 = '$host/files/audio/coins.wav';
final wavUrl2 = '$host/files/audio/laser.wav';
final wavUrl3 = '$host/files/audio/coins_non_ascii_и.wav';
final mp3Url1 = '$host/files/audio/ambient_c_motion.mp3';
final mp3Url2 = '$host/files/audio/nasa_on_a_mission.mp3';
final m3u8StreamUrl = useLocalServer
Expand All @@ -30,6 +31,8 @@ final mpgaStreamUrl = useLocalServer
const wavAsset = 'laser.wav';
const mp3Asset = 'nasa_on_a_mission.mp3';
const invalidAsset = 'invalid.txt';
const specialCharAsset = 'coins_non_ascii_и.wav';
const noExtensionAsset = 'coins_no_extension';

class SourcesTab extends StatefulWidget {
final AudioPlayer player;
Expand Down
6 changes: 5 additions & 1 deletion packages/audioplayers/example/server/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ <h1>Audioplayers Test Server</h1>
href="https://pub.dev/packages/audioplayers">Audioplayers</a>
package:</p>
<ul>
<li><a href="/files/audio/laser.wav">/files/audio/ambient_c_motion.mp3</a></li>
<li><a href="/files/audio/ambient_c_motion.mp3">/files/audio/ambient_c_motion.mp3</a></li>
<li><a href="/files/audio/coins.wav">/files/audio/coins.wav</a></li>
<li><a href="/files/audio/coins%20whitespace.wav">/files/audio/coins whitespace.wav</a></li>
<li><a href="/files/audio/coins_no_extension">/files/audio/coins_no_extension</a></li>
<li><a href="/files/audio/coins_non_ascii_%D0%B8.wav">/files/audio/coins_non_ascii_и.wav</a></li>
<li><a href="/files/audio/invalid.txt">/files/audio/invalid.txt</a></li>
<li><a href="/files/audio/laser.wav">/files/audio/laser.wav</a></li>
<li><a href="/files/audio/nasa_on_a_mission.mp3">/files/audio/nasa_on_a_mission.mp3</a></li>
<li>
Expand Down
29 changes: 23 additions & 6 deletions packages/audioplayers/lib/src/audio_cache.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';

import 'package:audioplayers/src/uri_ext.dart';
import 'package:file/file.dart';
import 'package:file/local.dart';
import 'package:flutter/foundation.dart';
Expand All @@ -21,7 +22,7 @@ import 'package:path_provider/path_provider.dart';
/// For most normal uses, the static instance is used. But if you want to
/// control multiple caches, you can create your own instances.
class AudioCache {
/// A globlally accessible instance used by default by all players.
/// A globally accessible instance used by default by all players.
static AudioCache instance = AudioCache();

@visibleForTesting
Expand Down Expand Up @@ -102,20 +103,35 @@ class AudioCache {
return tryAbsolute!;
}

// local asset
return Uri.parse('assets/$prefix$fileName');
// Relative Asset path
// URL-encode twice, see:
// https://github.com/flutter/engine/blob/2d39e672c95efc6c539d9b48b2cccc65df290cc4/lib/web_ui/lib/ui_web/src/ui_web/asset_manager.dart#L61
// Parsing an already encoded string to an Uri does not encode it a second
// time, so we have to do it manually:
final encoded = UriCoder.encodeOnce(fileName);
return Uri.parse(Uri.encodeFull('assets/$prefix$encoded'));
}

/// Loads a single [fileName] to the cache.
///
/// Also returns a [Future] to access that file.
/// Returns a [Uri] to access that file.
Future<Uri> load(String fileName) async {
if (!loadedFiles.containsKey(fileName)) {
loadedFiles[fileName] = await fetchToMemory(fileName);
}
return loadedFiles[fileName]!;
}

/// Loads a single [fileName] to the cache.
///
/// Returns a decoded [String] to access that file.
Future<String> loadPath(String fileName) async {
final encodedPath = (await load(fileName)).path;
// Web needs an url double-encoded path.
// Darwin needs a decoded path for local files.
return kIsWeb ? encodedPath : Uri.decodeFull(encodedPath);
}

/// Loads a single [fileName] to the cache but returns it as a File.
///
/// Note: this is not available for web, as File doesn't make sense on the
Expand All @@ -125,8 +141,9 @@ class AudioCache {
throw 'This method cannot be used on web!';
}
final uri = await load(fileName);
return fileSystem.file(uri.toFilePath(
windows: defaultTargetPlatform == TargetPlatform.windows));
return fileSystem.file(
uri.toFilePath(windows: defaultTargetPlatform == TargetPlatform.windows),
);
}

/// Loads a single [fileName] to the cache but returns it as a list of bytes.
Expand Down
12 changes: 9 additions & 3 deletions packages/audioplayers/lib/src/audioplayer.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:async';

import 'package:audioplayers/audioplayers.dart';
import 'package:audioplayers/src/uri_ext.dart';
import 'package:audioplayers_platform_interface/audioplayers_platform_interface.dart';
import 'package:flutter/services.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -325,8 +326,13 @@ class AudioPlayer {
Future<void> setSourceUrl(String url) async {
_source = UrlSource(url);
await creatingCompleter.future;
// Encode remote url to avoid unexpected failures.
await _completePrepared(
() => _platform.setSourceUrl(playerId, url, isLocal: false),
() => _platform.setSourceUrl(
playerId,
UriCoder.encodeOnce(url),
isLocal: false,
),
);
}

Expand All @@ -349,10 +355,10 @@ class AudioPlayer {
/// this method.
Future<void> setSourceAsset(String path) async {
_source = AssetSource(path);
final url = await audioCache.load(path);
final cachePath = await audioCache.loadPath(path);
await creatingCompleter.future;
await _completePrepared(
() => _platform.setSourceUrl(playerId, url.path, isLocal: true),
() => _platform.setSourceUrl(playerId, cachePath, isLocal: true),
);
}

Expand Down
10 changes: 10 additions & 0 deletions packages/audioplayers/lib/src/uri_ext.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
extension UriCoder on Uri {
static String encodeOnce(String uri) {
var tmpUri = uri;
try {
// Try decoding first to avoid encoding twice:
tmpUri = Uri.decodeFull(tmpUri);
} on ArgumentError catch (_) {}
return Uri.encodeFull(tmpUri);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import xyz.luan.audioplayers.player.SoundPoolManager
import xyz.luan.audioplayers.player.WrappedPlayer
import xyz.luan.audioplayers.source.BytesSource
import xyz.luan.audioplayers.source.UrlSource
import java.io.FileNotFoundException
import java.lang.ref.WeakReference
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.ConcurrentMap
Expand Down Expand Up @@ -122,7 +123,17 @@ class AudioplayersPlugin : FlutterPlugin, IUpdateCallback {
"setSourceUrl" -> {
val url = call.argument<String>("url") ?: error("url is required")
val isLocal = call.argument<Boolean>("isLocal") ?: false
player.source = UrlSource(url, isLocal)
try {
player.source = UrlSource(url, isLocal)
} catch (e: FileNotFoundException) {
response.error(
"AndroidAudioError",
"Failed to set source. For troubleshooting, see: " +
"https://github.com/bluefireteam/audioplayers/blob/main/troubleshooting.md",
e,
)
return
}
}

"setSourceBytes" -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,6 @@ class WrappedPlayer internal constructor(
}

fun onError(what: Int, extra: Int): Boolean {
// When an error occurs, reset player to not [prepared].
// Then no functions will be called, which end up in an illegal player state.
prepared = false
val whatMsg = if (what == MediaPlayer.MEDIA_ERROR_SERVER_DIED) {
"MEDIA_ERROR_SERVER_DIED"
} else {
Expand All @@ -323,7 +320,19 @@ class WrappedPlayer internal constructor(
MediaPlayer.MEDIA_ERROR_TIMED_OUT -> "MEDIA_ERROR_TIMED_OUT"
else -> "MEDIA_ERROR_UNKNOWN {extra:$extra}"
}
handleError(whatMsg, extraMsg, null)
if (!prepared && extraMsg == "MEDIA_ERROR_SYSTEM") {
handleError(
"AndroidAudioError",
"Failed to set source. For troubleshooting, see: " +
"https://github.com/bluefireteam/audioplayers/blob/main/troubleshooting.md",
"$whatMsg, $extraMsg",
)
} else {
// When an error occurs, reset player to not [prepared].
// Then no functions will be called, which end up in an illegal player state.
prepared = false
handleError("AndroidAudioError", whatMsg, extraMsg)
}
return false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,10 @@ public class SwiftAudioplayersDarwinPlugin: NSObject, FlutterPlugin {
},
completerError: {
player.eventHandler.onError(
code: "DarwinAudioError", message: "AVPlayerItem.Status.failed on setSourceUrl",
details: nil)
code: "DarwinAudioError",
message: "Failed to set source. For troubleshooting, see "
+ "https://github.com/bluefireteam/audioplayers/blob/main/troubleshooting.md",
details: "AVPlayerItem.Status.failed on setSourceUrl")
})
result(1)
return
Expand Down
Loading

0 comments on commit a4d8442

Please sign in to comment.