Skip to content
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

Give better errors when materialization fails #4788

Conversation

deivid-rodriguez
Copy link
Member

What was the end-user or developer problem that led to this PR?

When materialization fails, the error user gets can be confusing because it only shows the first gem that failed to be materialized. Sometimes it can be a single gem that's missing, sometimes it can be the whole bundle. Showing only the first gem that fails to materialize can lead end users into thinking there's a specific issue with that single gem, so this PR changes bundler to show the complete list of gems that failed to be materialized.

What is your fix for the problem, implemented in this PR?

My fix is to refactor the current implementation to be able to show the full list of gems that failed to be materialized. It's a bit more work, but it only happens on errors, so definitely not a critical code path, and as a result it allows us to simplify code in Bundler::Definition (our main "god class") and in Bundler::SpecSec (another complicated class).

In addition, I added a bundle install hint for when this error happens when running bundle list before bundle install, like we do in other cases like bundler/setup or bundle check.

I made this work while working on #4782, but I think it's justified as a separate improvement.

Make sure the following tasks are checked

This error can only be raised when loading the cache, and we only load
the cache if this condition is met.
Not only the first one that's missing.

This also allows us to simplify things.
If we have succeeded to materialize the bundle, all specs should be
loaded.
Bundler formatters already take care of this.
@deivid-rodriguez deivid-rodriguez merged commit 677213e into master Jul 28, 2021
@deivid-rodriguez deivid-rodriguez deleted the show_all_missing_gems_when_using_a_bundle_before_installing_it branch July 28, 2021 06:54
deivid-rodriguez added a commit that referenced this pull request Jul 30, 2021
…ng_a_bundle_before_installing_it

Give better errors when materialization fails

(cherry picked from commit 677213e)
deivid-rodriguez added a commit that referenced this pull request Jul 30, 2021
…ng_a_bundle_before_installing_it

Give better errors when materialization fails

(cherry picked from commit 677213e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants