[CLOSED] Fix 3174 (unknown internal error when installing non-zip file)

[core-ai-bot]

Issue by dangoor
Wednesday Mar 20, 2013 at 02:28 GMT
Originally opened as adobe/brackets#3179

---

Fixes #3174: Changed Package.formatError to better handle the error messages returned from the server

---

dangoor included the following code: https://github.com/adobe/brackets/pull/3179/commits

[core-ai-bot]

Comment by dangoor
Wednesday Mar 20, 2013 at 02:29 GMT

---

to@peterflynn

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 20, 2013 at 22:05 GMT

---

The pull request NJ just merge contains a different fix for this -- in Package.installFromURL(), we just pull out the first validation error, so clients of Package (i.e. callers of formatError()) are never exposed to the nested array case.

I originally favored a fix like this, but I was concerned that it leaves installFromURL()'s error format somewhat ambiguous. If you get ["foo", ... ], is it an array of errors, where the first error happens to have no format() params (i.e. it's just a flat string), or is it a single error with format() params? It just so happens that the first case never arises in our code -- multiple errors only occur with validation errors, none of those are flat strings (compared to e.g. disabledReason where all the errors are flat strings but there's never > 1 of them).

Seems like it's worth (maybe after this sprint) doing some cleanup of our error format to resolve several issues:

• Whether validation errors should be passed back as an error arg vs. in the success object (per our thread in [CLOSED] Corrected some mistakes in Polish translation #3158). And the similar, maybe even trickier question for disabledReason.
• Whether the error arg should be an object of type Error. We'd need to fix ConnectionManager to serialize it properly, and have some sort of subclass or convention to allow passing data beyond the standard message & stack (e.g. an error code & its formatting info).
• How to return multiple validation errors in a format that's not ambiguous (e.g. disallow flat-string error codes, or always return singleton errors objects wrapped in a length-1 array).

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 20, 2013 at 22:06 GMT

---

So I guess what I'm saying is... as long as #3174 really is fixed by pull #3178, maybe we should just spin off a cleanup bug and then close this?

[core-ai-bot]

Comment by peterflynn
Wednesday Mar 20, 2013 at 22:08 GMT

---

And yep, looks like that pull did fix it. I'll FBNC it to Glenn to make sure...

[core-ai-bot]

Comment by dangoor
Thursday Mar 21, 2013 at 00:52 GMT

---

@peterflynn I agree, it would be good to try to make our error formats as consistent as possible. As I mentioned in the issue for node connections, I think we want to take extensions into account and I think it's entirely possible that we'd end up with a slightly different looking abstraction.

[core-ai-bot]

Comment by peterflynn
Thursday Mar 21, 2013 at 21:33 GMT

---

I spun off #3205 with a summary of the questions raised here --@dangoor please chime in there if I've left anything out. Seems like it's ok to close this pull request now.