Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Fix the bug where errors during installation of a local .zip file wer… - #12702

Merged
petetnt merged 3 commits into
masterfrom
marcel/fix-8498
Aug 22, 2016
Merged

petetnt merged 3 commits into
masterfrom
marcel/fix-8498

Conversation

@marcelgerber

Copy link
Copy Markdown
Contributor

…e always shown as "Unknown internal error"

Fixes #8498.

info.errors is an array in the format [error1, error2], where every error is another array in the format [errorCode, additionalInfo1, additionalInfo2], so overall, info.errors could look like [["MISSING_PACKAGE_NAME", "C:/ext.zip"], ["INVALID_VERSION_NUMBER", "invalidver", "C/ext.zip"]]. Package.formatError, though, can only handle formatting one error.
That's why I decided we only handle the first error in the array (we can only output one error per file), so we don't always get "Unknown internal error" when attempting to install an invalid local .zip file.

…e always shown as "Unknown internal error"
@petetnt

petetnt commented Aug 22, 2016

Copy link
Copy Markdown
Collaborator

LGTM, but would prefer if Package.formatError(/s?) would format all the errors by instead of just picking up the first one.

@marcelgerber

Copy link
Copy Markdown
Contributor Author

In this case, I'm not quite sure what the UI should look like.
At this point, we have a list of * filename: error message (we need it be a list as in update/uninstall scenarios, several extensions can fail at once).
I think it would look odd if one extension was listed several times.

@petetnt

petetnt commented Aug 22, 2016

Copy link
Copy Markdown
Collaborator

I would be okay with just them being separated with a dot, as long as the information doesn't go missing.

@marcelgerber

Copy link
Copy Markdown
Contributor Author

That works, I guess.
Looks like this now:
image

Also added the MISSING_PACKAGE_JSON string because we didn't have one before.

@petetnt

petetnt commented Aug 22, 2016

Copy link
Copy Markdown
Collaborator

LGTM, merging this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants