refactor: Abstract lyric parsing & make try-catch reusable #17
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package com.cyanchill.missingcore.metadataretriever.utils | ||
|
|
||
| import androidx.annotation.OptIn | ||
| import androidx.media3.common.Metadata | ||
| import androidx.media3.common.util.UnstableApi | ||
| import androidx.media3.extractor.metadata.id3.BinaryFrame | ||
| import androidx.media3.extractor.metadata.id3.TextInformationFrame | ||
| import androidx.media3.extractor.metadata.vorbis.VorbisComment | ||
|
|
||
| @OptIn(UnstableApi::class) | ||
| class LyricsParser(entry: Metadata.Entry) { | ||
| var lyrics: String? = null | ||
| var isSync = false | ||
|
|
||
| init { | ||
| // Extra lyrics based on the class. | ||
| when (entry) { | ||
| is TextInformationFrame -> handleTextInformationFrame(entry) | ||
| is BinaryFrame -> handleBinaryFrame(entry) | ||
| is VorbisComment -> handleVorbisComment(entry) | ||
| } | ||
|
|
||
| // Sanitize input | ||
| lyrics = lyrics?.replace("\u0000", "") | ||
| } | ||
|
|
||
| private fun handleTextInformationFrame(frame: TextInformationFrame) { | ||
| val tagName = frame.id.uppercase() | ||
| if (tagName !in ID3v2_LYRIC_TAGS) return | ||
| lyrics = frame.values.firstOrNull() | ||
| isSync = tagName == "SYLT" | ||
| } | ||
|
|
||
| private fun handleBinaryFrame(frame: BinaryFrame) { | ||
| val tagName = frame.id.uppercase() | ||
| if (tagName !in ID3v2_LYRIC_TAGS) return | ||
|
|
||
| val byteArr = frame.data | ||
| // The 1st byte in the array determines the encoding in ID3. | ||
| // - Mp3Tag doesn't specify a Byte Order Mark if it's `1` (UTF-16), so we'll do a heuristic guess for what charset to use. | ||
| // - Ref: https://mutagen-specs.readthedocs.io/en/latest/id3/id3v2.4.0-structure.html#id3v2-frame-overview | ||
| val encodingCharSet = when (byteArr[0].toString()) { | ||
| "0" -> Charsets.ISO_8859_1 | ||
| "1" -> { | ||
| if (byteArr[1] == BYTE_0xFE && byteArr[2] == BYTE_0xFF) { | ||
| Charsets.UTF_16BE | ||
| } else if (byteArr[1] == BYTE_0xFF && byteArr[2] == BYTE_0xFE) { | ||
| Charsets.UTF_16LE | ||
| } else { | ||
| // Heuristic guess of byte order using new line character. | ||
| // - If the conversion contains a newline character, that charset should be used. | ||
| if (String(byteArr, Charsets.UTF_16LE).contains("\n")) Charsets.UTF_16LE | ||
| else Charsets.UTF_16BE | ||
| } | ||
| } | ||
| "2" -> Charsets.UTF_16BE | ||
| else -> Charsets.UTF_8 | ||
| } | ||
|
|
||
| lyrics = String(byteArr, encodingCharSet) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| isSync = tagName == "SYLT" | ||
| } | ||
|
|
||
| private fun handleVorbisComment(comment: VorbisComment) { | ||
| if (comment.key.uppercase() != "LYRICS") return | ||
| // We immediately bail out if we find lyrics in Vorbis Comments due to | ||
| // there being no specific tag for synchronized lyrics. | ||
| lyrics = comment.value | ||
| isSync = true | ||
| } | ||
|
|
||
| companion object { | ||
| private val ID3v2_LYRIC_TAGS = listOf("SYLT", "USLT") | ||
| private const val BYTE_0xFE = 0xFE.toByte() | ||
| private const val BYTE_0xFF = 0xFF.toByte() | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package com.cyanchill.missingcore.metadataretriever.utils | ||
|
|
||
| import com.facebook.react.bridge.Promise | ||
| import java.util.concurrent.ExecutionException | ||
|
|
||
| /** | ||
| * Reusable try/catch wrapper for when we do something pertaining to a file URI. Code that executes | ||
| * in the "try" portion goes in the provided `block`. | ||
| */ | ||
| fun <T> safeExecuteOnURI(uri: String, errorKey: String, promise: Promise, block: () -> T) { | ||
| try { | ||
| block() | ||
| } catch (e: ExecutionException) { | ||
| val isWantedException = | ||
| e.message?.contains("androidx.media3.datasource.FileDataSource\$FileDataSourceException") | ||
| ?: false | ||
| when (isWantedException) { | ||
| true -> promise.reject("ENOENT", "ENOENT: No such file or directory (${uri})", e) | ||
| false -> promise.reject(errorKey, e.message, e) | ||
|
Comment on lines
+13
to
+19
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# 1) Inspect the Kotlin file around the reported lines
FILE="android/src/main/java/com/cyanchill/missingcore/metadataretriever/utils/SafeUtils.kt"
echo "=== File exists? ==="
ls -l "$FILE" || true
echo
echo "=== Kotlin snippet (lines 1-120) ==="
nl -ba "$FILE" | sed -n '1,120p'
# 2) Locate safeExecuteOnURI and related usage
echo
echo "=== Search for safeExecuteOnURI ==="
rg -n "safeExecuteOnURI" -S . || true
# 3) Find JS mapping where error.name is set/used
echo
echo "=== Search for error.name mapping ==="
rg -n "error\.name" -S src . || true
# 4) If src/index.ts exists, show relevant area around the cited lines
IDX="src/index.ts"
if [ -f "$IDX" ]; then
echo
echo "=== src/index.ts snippet (lines 1-120) ==="
nl -ba "$IDX" | sed -n '1,160p'
echo
echo "=== src/index.ts around 27-60 ==="
nl -ba "$IDX" | sed -n '20,80p'
else
echo
echo "=== No src/index.ts found at $IDX ==="
ls -la src || true
fiRepository: MissingCore/react-native-metadata-retriever Length of output: 341 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="android/src/main/java/com/cyanchill/missingcore/metadataretriever/utils/SafeUtils.kt"
echo "=== Tool availability ==="
for t in rg grep awk sed cat ls head tail find xargs; do
command -v "$t" >/dev/null 2>&1 && echo "$t: yes" || echo "$t: no"
done
echo
echo "=== File exists? ==="
ls -l "$FILE" || true
echo
echo "=== Kotlin snippet (lines 1-180) with line numbers (awk) ==="
awk 'NR>=1 && NR<=180 {printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,180p'
echo
echo "=== Search for safeExecuteOnURI ==="
if command -v rg >/dev/null 2>&1; then
rg -n "safeExecuteOnURI" -S . || true
else
grep -RIn "safeExecuteOnURI" . || true
fi
echo
echo "=== Search for error.name mapping ==="
if command -v rg >/dev/null 2>&1; then
rg -n "error\.name" -S src . || true
else
grep -RIn "error\.name" src . || true
fi
echo
IDX="src/index.ts"
if [ -f "$IDX" ]; then
echo "=== src/index.ts snippet (lines 1-220) with line numbers (awk) ==="
awk 'NR>=1 && NR<=220 {printf "%5d\t%s\n", NR, $0}' "$IDX" | sed -n '1,220p'
echo
echo "=== src/index.ts around 20-90 ==="
awk 'NR>=20 && NR<=90 {printf "%5d\t%s\n", NR, $0}' "$IDX"
else
echo "=== No src/index.ts found at $IDX ==="
ls -la src || true
fiRepository: MissingCore/react-native-metadata-retriever Length of output: 8914 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect MetadataRetrieverModule where safeExecuteOnURI is called and where promise.reject errors are handled
FILE1="android/src/main/java/com/cyanchill/missingcore/metadataretriever/modules/MetadataRetrieverModule.kt"
echo "=== Kotlin snippet: $FILE1 (lines 1-360) ==="
awk 'NR>=1 && NR<=360 {printf "%5d\t%s\n", NR, $0}' "$FILE1" | sed -n '1,360p'
# Search for promise.reject usage
echo
echo "=== Search for promise.reject in MetadataRetrieverModule.kt ==="
rg -n "promise\.reject" "$FILE1" || true
# Search for how errors are serialized back to JS (look for map/Bundle with keys like 'code'/'message'/'name'/'data'/'errors')
echo
echo "=== Search for errors array construction in TS ==="
# try find MetadataRetriever implementation
for f in src/MetadataRetriever.ts src/MetadataRetriever.* src/**/MetadataRetriever*.ts; do :; done
# find files containing "getBulkMetadata" and "result.errors"
rg -n "getBulkMetadata|result\.errors|errors\]|\berrors\b" -S src || true
echo
echo "=== Search for NativeError / error data shape in TS ==="
rg -n "Native|reject|code|message|data|error\.name|error\.code" -S src || trueRepository: MissingCore/react-native-metadata-retriever Length of output: 21922 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Show src/types/GetMetadata.ts ==="
FILE="src/types/GetMetadata.ts"
ls -l "$FILE" || true
awk 'NR>=1 && NR<=120 {printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,120p'
echo
echo "=== Show src/NativeMetadataRetriever.ts (top) ==="
FILE="src/NativeMetadataRetriever.ts"
awk 'NR>=1 && NR<=140 {printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,140p'
echo
echo "=== Show src/MetadataRetriever.ts (top) ==="
FILE="src/MetadataRetriever.ts"
awk 'NR>=1 && NR<=160 {printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,160p'
echo
echo "=== Search for Android promise.reject serialization ==="
# If there is a shared ErrorObject or mapping on native side.
rg -n "promise\.reject|ErrorObject|ResultObject\(" android/src/main/java -S || true
echo
echo "=== Show NativeMetadataRetrieverSpec.kt (top if exists) ==="
SPEC="android/src/main/java/com/cyanchill/missingcore/metadataretriever/NativeMetadataRetrieverSpec.java"
SPEC2="android/src/main/java/com/cyanchill/missingcore/metadataretriever/NativeMetadataRetrieverSpec.kt"
if [ -f "$SPEC2" ]; then
awk 'NR>=1 && NR<=220 {printf "%5d\t%s\n", NR, $0}' "$SPEC2" | sed -n '1,220p'
elif [ -f "$SPEC" ]; then
awk 'NR>=1 && NR<=220 {printf "%5d\t%s\n", NR, $0}' "$SPEC" | sed -n '1,220p'
else
echo "Spec files not found at expected paths."
# try find by name
fd -t f "NativeMetadataRetrieverSpec.*" android/src/main/java -a || true
fiRepository: MissingCore/react-native-metadata-retriever Length of output: 6012 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="android/src/main/java/com/cyanchill/missingcore/metadataretriever/models/BridgeReturnables.kt"
echo "=== Show $FILE ==="
awk 'NR>=1 && NR<=200 {printf "%5d\t%s\n", NR, $0}' "$FILE" | sed -n '1,200p'
echo
# Ensure MetadataRetrieverModule already has brittle check; show that region
FILE2="android/src/main/java/com/cyanchill/missingcore/metadataretriever/modules/MetadataRetrieverModule.kt"
echo "=== Show getBulkMetadata catch region (lines ~120-170) ==="
awk 'NR>=120 && NR<=180 {printf "%5d\t%s\n", NR, $0}' "$FILE2" | sed -n '120,180p'
echo
# Search for promise rejection handling on JS side (ErrorObject / error mapping for rejected promises)
echo "=== Search TS for 'ENOENT' and for handling rejected native errors ==="
rg -n "ENOENT|ERR_" src . || trueRepository: MissingCore/react-native-metadata-retriever Length of output: 2353 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== List src directory files (top 200) ==="
find src -maxdepth 2 -type f | sed -n '1,200p'
echo
echo "=== Search for getArtwork/getLyric/getR128Gain error handling ==="
rg -n "getArtwork|getLyric|getR128Gain|ENOENT|ERR_" src --hidden || true
echo
echo "=== Show src/MetadataRetriever.ts if it exists ==="
FILE="src/MetadataRetriever.ts"
if [ -f "$FILE" ]; then
awk 'NR>=1 && NR<=200 {printf "%5d\t%s\n", NR, $0}' "$FILE"
else
echo "No $FILE"
ls -ლა src
fiRepository: MissingCore/react-native-metadata-retriever Length of output: 2342 Map ENOENT via the exception cause chain instead of
Suggested direction } catch (e: ExecutionException) {
- val isWantedException =
- e.message?.contains("androidx.media3.datasource.FileDataSource\$FileDataSourceException")
- ?: false
+ val isWantedException =
+ generateSequence(e as Throwable?) { it.cause }
+ .any { it?.javaClass?.name == "androidx.media3.datasource.FileDataSource\$FileDataSourceException" }
when (isWantedException) {
true -> promise.reject("ENOENT", "ENOENT: No such file or directory (${uri})", e)
false -> promise.reject(errorKey, e.message, e)
}🤖 Prompt for AI Agents |
||
| } | ||
| } catch (e: Exception) { | ||
| promise.reject(errorKey, e.message, e) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard short binary frames before reading bytes 0/1/2.
A malformed or truncated lyric frame will throw
ArrayIndexOutOfBoundsExceptionhere. SincegetLyricnow runs insidesafeExecuteOnURI, one bad embedded tag turns the whole call into a rejected Promise instead of falling back to other entries or returningnull.Suggested fix
🤖 Prompt for AI Agents