-
Notifications
You must be signed in to change notification settings - Fork 86
Condense Exception traceback when import torchcodec fails
#1153
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,10 +63,56 @@ def load_torchcodec_shared_libraries() -> tuple[int, str]: | |
| pybind_ops_module_name, pybind_ops_library_path | ||
| ) | ||
| return ffmpeg_major_version, core_library_path | ||
| except Exception: | ||
| # Capture the full traceback for this exception | ||
| exc_traceback = traceback.format_exc() | ||
| exceptions.append((ffmpeg_major_version, exc_traceback)) | ||
| except Exception as e: | ||
| # Below: all this block is just about trying to provide a more | ||
| # condensed and more informative exception message to the user, | ||
| # instead of dumping the entire traceback for each FFmpeg version, | ||
| # which would be too verbose. | ||
| # TODO: we really need a decent log system with different verbosity | ||
| # levels instead of this. | ||
| if isinstance(e, ImportError) and "No spec found for libtorchcodec" in str( | ||
| e | ||
| ): | ||
| # This should only happen when building from source for a single | ||
| # target FFmpeg version. | ||
| exceptions.append( | ||
| ( | ||
| ffmpeg_major_version, | ||
| f"Could not find one of the libtorchcodec* libraries, probably because TorchCodec wasn't built for FFmpeg {ffmpeg_major_version}.\n", | ||
| ) | ||
| ) | ||
| else: | ||
| full_traceback_str = traceback.format_exc() | ||
| # If we get something like this: | ||
| # OSError: Could not load this library: [...]torchcodec/src/torchcodec/libtorchcodec_core6.so | ||
| # Then in the traceback we try to find a line like this: | ||
| # OSError: libavcodec.so.60: cannot open shared object file: No such file or directory | ||
| # which should robustly indicate that the corresponding FFmpeg | ||
| # version is just not installed, or can't be found. | ||
| missing_ffmpeg_lib = next( | ||
| ( | ||
| line.strip() | ||
| for line in full_traceback_str.splitlines() | ||
| if "libav" in line and "No such file or directory" in line | ||
| ), | ||
| None, | ||
| ) | ||
| if ( | ||
| isinstance(e, OSError) | ||
| and ("Could not load this library") in str(e) | ||
| and "libtorchcodec" in (str(e)) | ||
| and missing_ffmpeg_lib | ||
| ): | ||
| exceptions.append( | ||
| ( | ||
| ffmpeg_major_version, | ||
| f"Got the following exception: {missing_ffmpeg_lib}\n" | ||
| f"FFmpeg version {ffmpeg_major_version} is likely not installed or its libraries cannot be found on this system.\n", | ||
| ) | ||
| ) | ||
| else: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Were you able to test any import fails to hit this else case? I do not recall the particulars of the stack trace, but if a stack trace begins with an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the scenario you mention above where we have an OSError and then an another exception stacked on top of it, we will only condense the error message if that other exception is of the form
i.e. only if it directly relates to libav* libraries. In other words, we're only condensing this into this: Any other import error will not be condensed and will hit this |
||
| # We can't identify the issue, so we just return the full traceback. | ||
| exceptions.append((ffmpeg_major_version, full_traceback_str)) | ||
|
|
||
| traceback_info = ( | ||
| "\n[start of libtorchcodec loading traceback]\n" | ||
|
|
||
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.
To define
missing_ffmpeg_libabove, we search the error message line by line, but here we are searching the entire string for the two elements of the error message. Is there some reason for that?It seems
Could not load this libraryandlibtorchcodecappear on the same line, so reusing the pattern might help to clarify the conditions we check to add a condensed error message.