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

handle failed policy set-version #7848

Merged
merged 3 commits into from
Jan 31, 2020
Merged

Conversation

olingern
Copy link
Contributor

Summary

Right now policies set-version does not handle invalid scenarios where an invalid github token is provided or the request fails.

Test plan

Will add a test that mocks a 401 from Github's API and that the reporter correctly logs.

@@ -428,6 +433,13 @@ export default class RequestManager {
}
}

const server = res.caseless.get('server');

if (res.statusCode === 401 && server === "GitHub.com") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if a constant is worth using here. Request manager is probably too generic in some ways. Would be nice to have a Github only request manager so stuff like this could be avoided. For now, I don't think this is terrible.

});
} catch (e) {
reporter.error(e.message);
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the event of a promise rejection, report and return early

@buildsize
Copy link

buildsize bot commented Jan 27, 2020

File name Previous Size New Size Change
yarn-[version].noarch.rpm 1.18 MB 1.18 MB 66 bytes (0%)
yarn-[version].js 4.86 MB 4.86 MB 638 bytes (0%)
yarn-legacy-[version].js 5.05 MB 5.05 MB 638 bytes (0%)
yarn-v[version].tar.gz 1.19 MB 1.19 MB 984 bytes (0%)
yarn_[version]all.deb 870.08 KB 870.2 KB 128 bytes (0%)

@olingern olingern force-pushed the fix-set-version-upon-failure branch from f9f2c53 to 208e40a Compare January 27, 2020 18:01
@arcanis arcanis merged commit fa613f3 into master Jan 31, 2020
@arcanis
Copy link
Member

arcanis commented Jan 31, 2020

Thanks 👍

VincentBailly pushed a commit to VincentBailly/yarn that referenced this pull request Jun 10, 2020
* handle failed policy set-version

* linting

* handle instances where caseless is not defined on response
VincentBailly pushed a commit to VincentBailly/yarn that referenced this pull request Jun 10, 2020
* handle failed policy set-version

* linting

* handle instances where caseless is not defined on response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants