-
Notifications
You must be signed in to change notification settings - Fork 14
Improve missing package URL handling #109
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
Conversation
Impacted by #104 |
Fixes #107 |
The tests are failing. Are you going to update then before merging? |
Tests are failing due to #104 so wasn't planning on delaying this fix (hard failures on every dotnet project) to refactor those |
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.
Pull Request Overview
This PR enhances the ComponentDetection
class to skip invalid or missing packageUrl
values and adds defensive checks in makePackageUrl
to prevent runtime errors. It also includes new unit tests to cover null inputs.
- Added early returns and debug logs when a component or referrer lacks a valid
packageUrl
. - Wrapped URL construction in a try-catch and added a null/undefined guard.
- Introduced tests for
makePackageUrl
handling of null arguments and properties.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
componentDetection.ts | Added defensive checks for missing/invalid packageUrl and wrapped URL creation in try-catch with debug logging. |
componentDetection.test.ts | Added tests for makePackageUrl to verify behavior on null inputs. |
Comments suppressed due to low confidence (3)
componentDetection.ts:157
- makePackageUrl currently only guards against null or undefined packageUrlJson, but allows objects with null Scheme or Type to produce strings like 'null:null/'. Add checks to return an empty string when Scheme or Type are missing or not valid strings.
public static makePackageUrl(packageUrlJson: any): string {
componentDetection.ts:77
- [nitpick] The loop variable 'component' shadows the nested 'component.component' property, which can be confusing. Consider renaming the iterator variable (e.g., to 'detectedComponent') for clarity.
json.componentsFound.forEach(async (component: any) => {
componentDetection.ts:157
- [nitpick] Consider adding a JSDoc comment for makePackageUrl to describe its parameters, behavior, and the meaning of an empty-string return for invalid inputs.
public static makePackageUrl(packageUrlJson: any): string {
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Enhance ComponentDetection
to gracefully handle missing or invalid packageUrl
values and prevent runtime errors by adding defensive checks and improving debugging output.
- Skip processing components and referrers when
packageUrl
is missing or invalid, with explanatory debug logs - Wrap URL construction in
makePackageUrl
with null checks and try/catch to return an empty string on failure - Add unit tests covering null and undefined
packageUrlJson
inputs
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
componentDetection.ts | Added guards for missing/invalid packageUrl and enhanced makePackageUrl with null checks and error handling |
componentDetection.test.ts | Introduced tests for makePackageUrl handling of null/undefined inputs and added negative test cases |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
That looks much more robust. Thanks!
This pull request enhances the robustness of the
ComponentDetection
class by adding defensive programming measures to handle invalid or missingpackageUrl
values gracefully. The changes improve error handling and debugging capabilities to ensure the system can handle edge cases without crashing. The dependency submission API is keyed by package url so this scenario will prevent the dependency graph from having any information about this package.Fixes
TypeError: Cannot read properties of null (reading 'Scheme')
Seems to be a highly visible issue introduced with component detection V5.2.16 that introduced a new detector: microsoft/component-detection#1388
Before:
After (debug test):
Test 2: