fix(core): catch all lock file parsing/pruning errors and provide fallback #15158
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -527,7 +528,13 @@ function nestMappedPackages( | |||
| }); | |||
|
|
|||
| if (initialSize === nestedNodes.size) { | |||
| throw Error('Loop detected while pruning. Please report this issue.'); | |||
| output.error({ | |||
There was a problem hiding this comment.
Why is this reported so low? Shouldn't we have a try catch around parseLockFile?
There was a problem hiding this comment.
This error happens on pruning and although it should not fail anymore, we still want to show error log if it does so we can track it.
This is the only remaining place we anticipate that error might occur.
There was a problem hiding this comment.
I think we should catch even unanticipated errors.
Throwing an Error here is fine. But we should catch it higher up if the whole operation is not necessary (/ there is a workaround)
There was a problem hiding this comment.
This can be an error again right?
There was a problem hiding this comment.
The intention was to try not break the lock file creation, as even without a few packages missing the installation might still work.
But I guess it's safer to just return the root lock file.
meeroslav
force-pushed
the
fix-error-message
branch
from
2577caf to
bc2ffe9
Compare
| @@ -527,7 +528,13 @@ function nestMappedPackages( | |||
| }); | |||
|
|
|||
| if (initialSize === nestedNodes.size) { | |||
| throw Error('Loop detected while pruning. Please report this issue.'); | |||
| output.error({ | |||
There was a problem hiding this comment.
I think we should catch even unanticipated errors.
Throwing an Error here is fine. But we should catch it higher up if the whole operation is not necessary (/ there is a workaround)
meeroslav
changed the title
| @@ -527,7 +528,13 @@ function nestMappedPackages( | |||
| }); | |||
|
|
|||
| if (initialSize === nestedNodes.size) { | |||
| throw Error('Loop detected while pruning. Please report this issue.'); | |||
| output.error({ | |||
There was a problem hiding this comment.
This can be an error again right?
meeroslav
force-pushed
the
fix-error-message
branch
from
9b4c7bb to
492bf0c
Compare
…lback (#15158) Co-authored-by: Jason Jean <[email protected]> (cherry picked from commit 5d69c4e)
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
When npm stringification after pruning fails because some dependnecies could not be nested it throws an error, breaking the build.
Expected Behavior
Lock file stringification error should not break the build. We should rather return existing (potentially functional) lock file and notify user of potential issue and how to overcome it.
Related Issue(s)
Fixes #