fix: Improve error handling in whisper_transcription_controller#694
fix: Improve error handling in whisper_transcription_controller#694YadavAkhileshh wants to merge 6 commits intoAOSSIE-Org:devfrom
Conversation
…r transcription - Changed print() to log() with proper error context - Added segment index to identify which segment failed - Captured stack trace for debugging - Added snackbar to notify users when transcription errors occur
|
🎉 Welcome @YadavAkhileshh!
We appreciate your contribution! 🚀 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughReworks segment processing in the whisper transcription controller to iterate with segment indices, capture errors with stack traces and segment context, log per-segment failures, count failed segments, and show a single Transcription Warning snackbar when any segments fail. No public APIs changed. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as WhisperController
participant Parser as SegmentParser
participant Logger
participant UI as Snackbar
rect rgba(200,200,255,0.5)
User->>Controller: convertToLrc(transcriptionSegments)
end
Controller->>Parser: for each segment (index i) -> _parseTranscriptionSegment(segment)
alt parse succeeds
Parser-->>Controller: lrc line
Controller->>Controller: append lrc line
else parse throws
Parser-->>Controller: throws (e, stackTrace)
Controller->>Logger: log('Error at index i', error: e, stackTrace: stackTrace)
Controller->>Controller: failedSegments += 1
end
rect rgba(200,255,200,0.5)
Controller->>UI: if failedSegments > 0 -> customSnackbar(Transcription Warning, LogType.warning)
UI-->>User: display snackbar
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lib/controllers/whisper_transcription_controller.dart:
- Around line 73-84: In the catch block inside the transcription loop (the code
that currently logs errors with log(...) and calls customSnackbar), stop calling
customSnackbar per-segment; instead, collect failed segment indices (e.g., push
i or i+1 into a local List<int> failedSegments) and continue logging as before;
after the loop that invokes segment processing (the scope around the try/catch
that uses i), check if failedSegments is not empty and then call customSnackbar
once with a concise summary message (e.g., count and optional small list or
range of failed segment numbers) to inform the user of aggregated failures; keep
the detailed log(...) calls untouched so stack traces remain available for
debugging.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/controllers/whisper_transcription_controller.dart
🔇 Additional comments (2)
lib/controllers/whisper_transcription_controller.dart (2)
8-9: LGTM! Necessary imports for enhanced error handling.The added imports support the new logging type enumeration and user notification functionality.
64-66: LGTM! Index-based iteration enables precise error reporting.The refactoring from an enhanced for-loop to an index-based loop correctly enables segment index reporting in error messages, which was a key objective of this PR.
M4dhav
added
enhancement
| for (WhisperTranscribeSegment? segment in transcriptionSegments) { | ||
| for (int i = 0; i < transcriptionSegments.length; i++) { | ||
| try { | ||
| final segment = transcriptionSegments[i]; |
There was a problem hiding this comment.
Why these changes? Mentioning the specific index is unnecessary. If it fails, it probably won't be due to a specific segment
| customSnackbar( | ||
| 'Transcription Error', | ||
| 'Failed to process segment ${i + 1}. Some content may be missing.', | ||
| LogType.error, | ||
| ); |
There was a problem hiding this comment.
Snackbars should be user friendly. And like coderabbit mentioned, users should not be spammed with them
|
@M4dhav @coderabbitai Thanks for the feedback! I've addressed the concerns: ✅ Reverted to original for-in loop style (removed index tracking) |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3997
However, I noticed one important discrepancy with the original requirements from issue Missing segment index in error logs 🔍 The current error log (lines 77-81) doesn't include which segment failed: log(
'Error converting transcription segment: ${e.toString()}',
error: e,
stackTrace: stackTrace,
);Issue Suggestion: You can track the index while maintaining the clean for-in loop style using for (var entry in transcriptionSegments.asMap().entries) {
final index = entry.key;
final segment = entry.value;
try {
final segmentString = _parseTranscriptionSegment(segment);
if (segmentString != null) {
lrcContent.writeln(segmentString);
}
} catch (e, stackTrace) {
log(
'Error converting transcription segment at index $index: ${e.toString()}',
error: e,
stackTrace: stackTrace,
);
failedSegments++;
}
}This approach:
Was removing the segment index from logs intentional, or would you like to add it back? The rest of the implementation looks solid! |
|
Always open PRs to dev and not master |
There was a problem hiding this comment.
Revert the loop logic to like it was before, much simpler. No need to keep a count of the segments. Just use a boolean to check whether some part has failed.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| 'Transcription Warning', | ||
| 'Some parts of the transcription could not be processed.', |
There was a problem hiding this comment.
Fixed! Snackbar text is now localized
2 participants
Description
Fixed the print() statement issue in whisper_transcription_controller.dart
Changes
Fixes
Fixes #680
Testing
Checked the code follows existing patterns in the project. The snackbar uses the same customSnackbar widget used elsewhere in the app.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.