-
Notifications
You must be signed in to change notification settings - Fork 319
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
NPM error code ELSPROBLEMS #9551
Comments
Maybe it's similar to [1] [1] angular/angular-cli#28647 |
Independently of whether the project's setup can be fixed or not, it's probably a good idea to change As a side note, I'll probably do some |
I cannot tell. The error we receive has a different code (ELSPROBLEMS), no packages are reported as MISSING, and there also other packages marked as INVALID. |
IIUC, the dependency analysis fails completely because the execution of the |
Have you investigate outside of ORT whether this can be fixed, e.g. by the steps I linked above? |
I have tried this fix, but it did not help - the |
Also, we have two independent users who encounter this issue, so it is for that reason also unlikely, that is on the end-user side. |
Not sure how you define "on the end user side". The error comes from the |
Another idea which one could try out as a work-around is to see whether any of the different install stratigies works: |
@oheger-bosch can you tell whether |
But wouldn't you see this as a regression, since the same project could be analyzed with ORT before without any issues? Facts are:
|
Yes, but what's the point? |
@fviernau Yes, I found it directly in the top-level node_modules folder. |
From [1] it would be nice to see output of @oheger-bosch can you run this and share output? [1] https://stackoverflow.com/questions/78832352/react-test-error-referenceerror-global-is-not-defined |
I agree that the implementation should be lenient and if the response from
@fviernau I believe the point is that you focus on potential issues in the analyzed projects which gives the impressions that you are against the proposed lenient implementation.
@sschuberth I'd rather change the functions return type to include the issues than to add side-effects to the function. |
Not only, I just want to extend the view to also consider if the issue is 1) in the project setup or 2) in
I'm not against this per se. I'm trying to help find the root cause. Note: Before making the implementation lenient I believe it would be good to try to understand that. Looking at other package managers, I believe this is not completely something out of the ordinary, to require the underlying tool to successfully list its dependencies. For example, |
|
@oheger-bosch when you say "issue" here, do you mean there is an ORT |
In any case, I'd like to point out #9383 that I created recently. In the context of that, I believe we should collect as much information as possible about analyzer problems, including to forward any problems reported by underlying CLI tools as ORT |
Sorry, bad wording on my side. What happens is that an |
I am seeing a similar behavior from what appears to be the same (or at least related) issue. When I run
Which seems odd, because the command that supposedly fails works fine when I run it. I originally saw this in v40, and also when upgrading to 42.1.0. After seeing this post I went back and tried older versions - ORT worked successfully up through v39.0.0, then breaks with v40.0.0 |
maybe interresting: npm/cli#6856 (comment) |
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
If `npm list` returns with a non-zero exit code, do not throw an exception and consequently stop the analysis. Instead, collect the errors from `stderr` as `Issue`s and continue analyzing the project, because useful dependency information may still be available. Resolves oss-review-toolkit#9551 Signed-off-by: Marcel Bochtler <[email protected]>
If `npm list` returns with a non-zero exit code, do not throw an exception and consequently stop the analysis. Instead, collect the errors from `stderr` as `Issue`s and continue analyzing the project, because useful dependency information may still be available. Resolves oss-review-toolkit#9551. Signed-off-by: Marcel Bochtler <[email protected]>
In commit 03560a5, there have been major changes in the implementation of NPM. Specifically, a new
npm list
command is now executed instead of scanning thenode_modules
folder.It seems that this approach can sometimes fail with error messages like the following:
After upgrading to an ORT version containing this fix, some of our projects which were successful before suddenly have this issue. For them no dependencies are found. Unfortunately, these are internal projects which cannot be shared, and I do not know an easy way to reproduce the problem.
However, from what I see from the output of the
npm list
command executed locally, still a valid JSON structure is returned containing information about the modules. It just has an additional "error" property with the error messages shown above. So, maybe, ORT could be more lenient here and still process the JSON structure, even if the command returns an exit code indicating an error? It could still generate an issue (maybe of type WARN) if the "error" property is found.The text was updated successfully, but these errors were encountered: