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

Continue trying package recognizers even if an Exception has been encountered #3971

Open
JonoYang opened this issue Nov 6, 2024 · 1 comment

Comments

@JonoYang
Copy link
Member

JonoYang commented Nov 6, 2024

I am updating go-inspector to v0.5.0 and I am failing the test test_recognize_winexe (https://github.com/aboutcode-org/scancode-toolkit/blob/develop/tests/packagedcode/test_recognize.py#L134) with the exception raised by go-inspector saying "Failed to parse file: no valid pclntab found".

I don't think it should matter if go-inspector can't parse the file, we should continue and try the other recognizers instead of stopping.

JonoYang added a commit that referenced this issue Nov 6, 2024
…ition #3971

Signed-off-by: Jono Yang <jyang@nexb.com>
@AyanSinhaMahapatra
Copy link
Member

with the exception raised by go-inspector saying "Failed to parse file: no valid pclntab found".

Ideally we should be catching this exception in the DatafileHandler on the go inspector side (if this is coming from inside https://github.com/aboutcode-org/go-inspector/blob/main/src/go_inspector/binary.py#L482) and yield nothing instead of failing there.

These exceptions usually are catched and swallowed at https://github.com/aboutcode-org/scancode-toolkit/blob/develop/src/scancode/api.py#L268 which is also not ideal, we should probably add these to the resource errors instead too.

But since the DataFile handlers at https://github.com/aboutcode-org/scancode-toolkit/blob/develop/src/packagedcode/__init__.py#L52 have an order (because some overlap) I'm not entirely sure of unintended consequences here, but the test suite does not seem to fail so we can look at these when we find them. So this is probably fine in the short term to update go inspector, we can refactor this later (we also need to move the packagedata creation part from go inspector soon too), so let's keep the issue open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants