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

Refactored flow error checks #1953

Merged
merged 6 commits into from Mar 13, 2019
Merged
Diff settings

Always

Just for now

Copy path View file
@@ -215,6 +215,8 @@ export default async function set(
);
if (handleResult === 1) {
return 1;
} else if (handleResult instanceof Error) {
throw handleResult;
This conversation was marked as resolved by juliangruber

This comment has been minimized.

Copy link
@javivelasco

javivelasco Mar 13, 2019

Member

Why throw? Who's handling this?

} else {
console.log(
`${chalk.cyan('> Success!')} ${
@@ -367,7 +369,7 @@ type RemainingAssignAliasErrors = SetDifference<
function handleCreateAliasError<T>(
output: Output,
error: RemainingAssignAliasErrors | T
): 1 | T {
): 1 | Error | T {
This conversation was marked as resolved by juliangruber

This comment has been minimized.

Copy link
@javivelasco

javivelasco Mar 13, 2019

Member

This is incorrect IIRC. This function is deliberately implemented to return 1 (when the error is handled) or something else, but not an error. When it returns an error instance it means that there was a known error received that wasn't handled so if you check at the end of the pattern match you'll see the remaining values that should be handled.

if (error instanceof ERRORS.AliasInUse) {
output.error(
`The alias ${chalk.dim(
Copy path View file
@@ -325,20 +325,7 @@ export default async function main(
createArgs
);

if (
firstDeployCall instanceof WildcardNotAllowed ||
firstDeployCall instanceof CantSolveChallenge ||
firstDeployCall instanceof DomainConfigurationError ||
firstDeployCall instanceof DomainNotFound ||
firstDeployCall instanceof DomainPermissionDenied ||
firstDeployCall instanceof DomainsShouldShareRoot ||
firstDeployCall instanceof DomainValidationRunning ||
firstDeployCall instanceof DomainVerificationFailed ||
firstDeployCall instanceof SchemaValidationFailed ||
firstDeployCall instanceof TooManyCertificates ||
firstDeployCall instanceof TooManyRequests ||
firstDeployCall instanceof InvalidDomain
) {
if (firstDeployCall instanceof Error) {
This conversation was marked as resolved by juliangruber

This comment has been minimized.

Copy link
@javivelasco

javivelasco Mar 13, 2019

Member

This is touching a JS file, that was my point. Since we have no inference here, probably it's not a bad idea to keep them for now but It's fine if we remove them anyway, as you prefer

handleCreateDeployError(output, firstDeployCall);
return 1;
}
@@ -401,19 +388,7 @@ export default async function main(
paths,
createArgs
);
if (
secondDeployCall instanceof WildcardNotAllowed ||
secondDeployCall instanceof CantSolveChallenge ||
secondDeployCall instanceof DomainConfigurationError ||
secondDeployCall instanceof DomainNotFound ||
secondDeployCall instanceof DomainPermissionDenied ||
secondDeployCall instanceof DomainsShouldShareRoot ||
secondDeployCall instanceof DomainValidationRunning ||
secondDeployCall instanceof DomainVerificationFailed ||
secondDeployCall instanceof SchemaValidationFailed ||
secondDeployCall instanceof TooManyCertificates ||
secondDeployCall instanceof TooManyRequests
) {
if (secondDeployCall instanceof Error) {
handleCreateDeployError(output, secondDeployCall);
return 1;
}
Copy path View file
@@ -60,12 +60,7 @@ export default async function assignAlias(
prevDeployment.scale,
deployment.url
);
if (
result instanceof ERRORS.NotSupportedMinScaleSlots ||
result instanceof ERRORS.ForbiddenScaleMinInstances ||
result instanceof ERRORS.ForbiddenScaleMaxInstances ||
result instanceof ERRORS.InvalidScaleMinMaxRelation
) {
if (result instanceof Error) {
return result;
}

@@ -95,20 +90,7 @@ export default async function assignAlias(
if (alias.indexOf('.') !== -1 && !NOW_SH_REGEX.test(alias)) {
// Now the domain shouldn't be available and it might or might not belong to the user
const result = await setupDomain(output, client, alias, contextName);
if (
result instanceof ERRORS.DomainAlreadyExists ||
result instanceof ERRORS.DomainNotAvailable ||
result instanceof ERRORS.DomainNotFound ||
result instanceof ERRORS.DomainPermissionDenied ||
result instanceof ERRORS.DomainPurchasePending ||
result instanceof ERRORS.DomainServiceNotAvailable ||
result instanceof ERRORS.DomainVerificationFailed ||
result instanceof ERRORS.InvalidDomain ||
result instanceof ERRORS.SourceNotFound ||
result instanceof ERRORS.UnexpectedDomainPurchaseError ||
result instanceof ERRORS.UnsupportedTLD ||
result instanceof ERRORS.UserAborted
) {
if (result instanceof Error) {
return result;
}

@@ -125,19 +107,7 @@ export default async function assignAlias(
alias,
externalDomain
);
if (
record instanceof ERRORS.AliasInUse ||
record instanceof ERRORS.CantSolveChallenge ||
record instanceof ERRORS.DeploymentNotFound ||
record instanceof ERRORS.DomainConfigurationError ||
record instanceof ERRORS.DomainPermissionDenied ||
record instanceof ERRORS.DomainsShouldShareRoot ||
record instanceof ERRORS.DomainValidationRunning ||
record instanceof ERRORS.InvalidAlias ||
record instanceof ERRORS.TooManyCertificates ||
record instanceof ERRORS.TooManyRequests ||
record instanceof ERRORS.InvalidDomain
) {
if (record instanceof Error) {
return record;
}

Copy path View file
@@ -37,16 +37,7 @@ export default async function createAlias(
alias,
!externalDomain
);
if (
cert instanceof ERRORS.CantSolveChallenge ||
cert instanceof ERRORS.DomainConfigurationError ||
cert instanceof ERRORS.DomainPermissionDenied ||
cert instanceof ERRORS.DomainsShouldShareRoot ||
cert instanceof ERRORS.DomainValidationRunning ||
cert instanceof ERRORS.TooManyCertificates ||
cert instanceof ERRORS.TooManyRequests ||
cert instanceof ERRORS.InvalidDomain
) {
if (cert instanceof Error) {
return cert;
}

@@ -8,10 +8,7 @@ export default async function getInferredTargets(
) {
// Read the configuration file from the best guessed location
const config = await getConfig(output, localConfigPath);
if (
config instanceof ERRORS.CantParseJSONFile ||
config instanceof ERRORS.CantFindConfig
) {
if (config instanceof Error) {
return config;
}

@@ -26,20 +26,7 @@ export default async function upsertPathAlias(

if (!NOW_SH_REGEX.test(alias)) {
const domainInfo = await setupDomain(output, client, alias, contextName);
if (
domainInfo instanceof ERRORS.DomainAlreadyExists ||
domainInfo instanceof ERRORS.DomainNotAvailable ||
domainInfo instanceof ERRORS.DomainNotFound ||
domainInfo instanceof ERRORS.DomainPermissionDenied ||
domainInfo instanceof ERRORS.DomainPurchasePending ||
domainInfo instanceof ERRORS.DomainServiceNotAvailable ||
domainInfo instanceof ERRORS.DomainVerificationFailed ||
domainInfo instanceof ERRORS.InvalidDomain ||
domainInfo instanceof ERRORS.SourceNotFound ||
domainInfo instanceof ERRORS.UnexpectedDomainPurchaseError ||
domainInfo instanceof ERRORS.UnsupportedTLD ||
domainInfo instanceof ERRORS.UserAborted
) {
if (domainInfo instanceof Error) {
return domainInfo;
}

@@ -60,15 +47,7 @@ export default async function upsertPathAlias(
alias,
!externalDomain
);
if (
cert instanceof ERRORS.CantSolveChallenge ||
cert instanceof ERRORS.DomainConfigurationError ||
cert instanceof ERRORS.DomainPermissionDenied ||
cert instanceof ERRORS.DomainsShouldShareRoot ||
cert instanceof ERRORS.DomainValidationRunning ||
cert instanceof ERRORS.TooManyCertificates ||
cert instanceof ERRORS.TooManyRequests
) {
if (cert instanceof Error) {
return cert;
}

@@ -57,13 +57,7 @@ export default async function purchaseDomainIfAvailable(

output.print(eraseLines(1));
const result = await purchaseDomain(client, domain, price);
if (
result instanceof ERRORS.SourceNotFound ||
result instanceof ERRORS.DomainNotAvailable ||
result instanceof ERRORS.DomainServiceNotAvailable ||
result instanceof ERRORS.InvalidDomain ||
result instanceof ERRORS.UnexpectedDomainPurchaseError
) {
if (result instanceof Error) {
return result;
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.