simplify error message when repo not found

[lancejpollard]

If duo-package doesn't resolve the repo, it gives a hard to debug error like this:

Error: { [Error: Command failed: /bin/sh -c git ls-remote --tags --heads https://{{token}}@github.com/some/component
remote: Repository not found.
fatal: repository 'https://{{token}}@github.com/some/component/' not found
]
  killed: false,
  code: 128,
  signal: null,
  cmd: '/bin/sh -c git ls-remote --tags --heads https://{{token}}@github.com/some/component' }

Maybe we could be nice and say something like this instead:

Package https://{{token}}@github.com/some/component not found.
Referenced in /Users/user/path/to/cwd/components/another/component/lib/index.js:12

That way it shows exactly where the problem was defined. Otherwise it takes 5-10 minutes sometimes to find where the problem is.

[lancejpollard]

To do this, we would have to:

1. Keep track of the lines that each of the dependencies came from (https://github.com/MatthewMueller/file-deps/blob/master/lib/js.js#L28).
2. Pass that info to the dep object in duo, which gets passed to Duo.prototype.dependency (https://github.com/component/duo/blob/master/lib/duo.js#L442).
3. Throw the nice error pointing to the original place in the file this occurs
   • Either pass that info to duo-package, where the error originates (https://github.com/component/duo-package/blob/master/lib/index.js#L215), or
   • try/catch and throw the nice error in duo (https://github.com/component/duo/blob/master/lib/duo.js#L468)

If we just passed the location in the file where the dependency was declared, all the errors would be very clear and straightforward. It would be cleaner probably if we had a full JS/CSS parser, but file-deps is nice because it's probably faster than parsing a full syntax tree (seems like you guys chose the non-syntax tree approach b/c of that).

Thoughts?

[lancejpollard]

To add line number to file-deps, all that would need to be done is split the input string, and match the patterns in there to each line rather than the whole file at once. Since we only support single line static statements anyways like require('x'), not require(\n'x'\n);.

[matthewmueller]

+1 on making the error nicer, but this approach seems to complicate the implementation quite a bit.

I don't think the line number matters as much as Referenced in /Users/user/path/to/cwd/components/another/component/lib/index.js.

I think we could accomplish this by printing out a slice of the half complete out when we encounter an error. Initial thoughts are to try-catch (https://github.com/component/duo/blob/master/lib/duo.js#L388) and report the error. Actually would be nice to not throw if package encounters an error. We could do that by passing a function to package.fetch(fn) (https://github.com/component/duo/blob/master/lib/duo.js#L468). When a fn is passed, package.fetch is no longer a generator.

Unfortunately, I still think the cleanliness will suffer from this change. So I'd like to have a clean approach before we take this on.

[ianstormtaylor]

It would already be an improvement just to report Could not find package ":owner/:repo" on GitHub. even without the source file that references it
I think?

On Thu, Aug 14, 2014 at 5:19 PM, Matthew Mueller [email protected]
wrote:

+1 on making the error nicer, but this approach seems to complicate the
implementation quite a bit.

I don't think the line number matters as much as Referenced in
/Users/user/path/to/cwd/components/another/component/lib/index.js.

I think we could accomplish this by printing out a slice of the half
complete out when we encounter an error. Initial thoughts are to
try-catch (https://github.com/component/duo/blob/master/lib/duo.js#L388)
and report the error. Actually would be nice to not throw if package
encounters an error. We could do that by passing a function to
package.fetch(fn) (
https://github.com/component/duo/blob/master/lib/duo.js#L468)

Unfortunately, I still think the cleanliness will suffer from this change.
So I'd like to have a clean approach before we take this on.

—
Reply to this email directly or view it on GitHub
#184 (comment).

[lancejpollard]

@ianstormtaylor that would be an improvement but it'd still be confusing, because you wouldn't know which component is requiring it, so you'd have to look through lots of components potentially

[ianstormtaylor]

True, but it's a better user experience that's quick to implement without
big re-archs, and then I'd just grep for where that thing is referenced.

On Thu, Aug 14, 2014 at 8:31 PM, Lance Pollard [email protected]
wrote:

@ianstormtaylor https://github.com/ianstormtaylor that would be an
improvement but it'd still be confusing, because you wouldn't know which
component is requiring it, so you'd have to look through lots of components
potentially

—
Reply to this email directly or view it on GitHub
#184 (comment).

[jruddell]

+1 on this, I understand it would make it harder to implement, but if its possible telling me where the error occurred would be huge