Skip to content

Commit

Permalink
fix: move throwable upgrades calculation back into try/catch
Browse files Browse the repository at this point in the history
Move throwable code into try/catch to make sure we continue fixing + spinner
is not stuck
Update a test to show that if some dependencies fail under --sequentially-fix
we can still update other dependencies.
  • Loading branch information
lili2311 committed Sep 14, 2021
1 parent 8fdc9b7 commit d820026
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 24 deletions.
Expand Up @@ -31,14 +31,14 @@ async function fixAll(
failed: [],
skipped: [],
};
const { upgrades } = await generateUpgrades(entity);
if (!upgrades.length) {
throw new NoFixesCouldBeAppliedError(
'Failed to calculate package updates to apply',
);
}
const changes: FixChangesSummary[] = [];
try {
const { upgrades } = await generateUpgrades(entity);
if (!upgrades.length) {
throw new NoFixesCouldBeAppliedError(
'Failed to calculate package updates to apply',
);
}
// TODO: for better support we need to:
// 1. parse the manifest and extract original requirements, version spec etc
// 2. swap out only the version and retain original spec
Expand Down
Expand Up @@ -3,12 +3,10 @@ import * as debugLib from 'debug';
import { PluginFixResponse } from '../../../../types';
import {
EntityToFix,
FixChangesError,
FixChangesSummary,
FixOptions,
} from '../../../../../types';
import { NoFixesCouldBeAppliedError } from '../../../../../lib/errors/no-fixes-applied';
import { isSuccessfulChange } from '../../attempted-changes-summary';
import { generateUpgrades } from './generate-upgrades';
import { poetryAdd } from './poetry-add';
import { ensureHasUpdates } from '../../ensure-has-updates';
Expand Down Expand Up @@ -95,8 +93,12 @@ async function fixSequentially(
// 2. swap out only the version and retain original spec
// 3. re-lock the lockfile
const changes: FixChangesSummary[] = [];

try {
if (![...upgrades, ...devUpgrades].length) {
throw new NoFixesCouldBeAppliedError(
'Failed to calculate package updates to apply',
);
}
// update prod dependencies first
if (upgrades.length) {
for (const upgrade of upgrades) {
Expand All @@ -113,7 +115,6 @@ async function fixSequentially(
);
}
}

ensureHasUpdates(changes);
handlerResult.succeeded.push({
original: entity,
Expand Down
Expand Up @@ -623,7 +623,7 @@ describe('fix Poetry Python projects fix sequentially', () => {
});

it('error is bubbled up', async () => {
jest.spyOn(poetryFix, 'poetryAdd').mockResolvedValue({
jest.spyOn(poetryFix, 'poetryAdd').mockResolvedValueOnce({
exitCode: 1,
stdout: '',
stderr: `Resolving dependencies... (1.7s)
Expand All @@ -633,7 +633,14 @@ describe('fix Poetry Python projects fix sequentially', () => {
Because package-A (2.6) depends on package-B (>=1.19)
and package-C (2.2.1) depends on package-B (>=1.16.0,<1.19.0), package-D (2.6) is incompatible with package-C (2.2.1).
So, because package-Z depends on both package-C (2.2.1) and package-D (2.6), version solving failed.`,
command: 'poetry install six==2.0.1 transitive==1.1.1',
command: 'poetry install six==2.0.1',
duration: 123,
});
jest.spyOn(poetryFix, 'poetryAdd').mockResolvedValueOnce({
exitCode: 0,
stdout: '',
stderr: '',
command: 'poetry install transitive==1.1.1',
duration: 123,
});

Expand Down Expand Up @@ -680,25 +687,45 @@ describe('fix Poetry Python projects fix sequentially', () => {
exceptions: {},
results: {
python: {
failed: [
failed: [],
skipped: [],
succeeded: [
{
original: entityToFix,
error: expect.objectContaining({
name: 'NoFixesCouldBeAppliedError',
}),
tip: 'Try running `poetry install six==2.0.1 transitive==1.1.1`',
changes: [
{
from: 'six@1.1.6',
issueIds: ['VULN-six'],
reason: `SolverProblemError
Because package-A (2.6) depends on package-B (>=1.19)
and package-C (2.2.1) depends on package-B (>=1.16.0,<1.19.0), package-D (2.6) is incompatible with package-C (2.2.1).
So, because package-Z depends on both package-C (2.2.1) and package-D (2.6), version solving failed`,
success: false,
tip: 'Try running `poetry install six==2.0.1`',
to: 'six@2.0.1',
userMessage: 'Failed to upgrade six from 1.1.6 to 2.0.1',
},
{
from: 'transitive@1.0.0',
issueIds: [],
success: true,
to: 'transitive@1.1.1',
userMessage: 'Pinned transitive from 1.0.0 to 1.1.1',
},
],
},
],
skipped: [],
succeeded: [],
},
},
});
expect(result.fixSummary).toContain('SolverProblemError');
expect(result.fixSummary).toContain('SolverProblemError');
expect(result.fixSummary).toContain(
'Tip: Try running `poetry install six==2.0.1 transitive==1.1.1`',
'Tip: Try running `poetry install six==2.0.1`',
);
expect(result.fixSummary).toContain(
'Pinned transitive from 1.0.0 to 1.1.1',
);
expect(result.fixSummary).toContain('✖ No successful fixes');
expect(poetryFixStub).toHaveBeenCalledTimes(2);
expect(poetryFixStub).toHaveBeenCalledWith(
pathLib.resolve(workspacesPath, 'simple'),
Expand Down
@@ -1,4 +1,4 @@
import { generateUpgrades } from '../../../../../../src/plugins/python/handlers/poetry/update-dependencies';
import { generateUpgrades } from '../../../../../../src/plugins/python/handlers/poetry/update-dependencies/generate-upgrades';
import { generateEntityToFix } from '../../../../../helpers/generate-entity-to-fix';
describe('generateUpgrades', () => {
it('returns empty if no upgrades could be generated', async () => {
Expand Down
1 change: 1 addition & 0 deletions src/cli/args.ts
Expand Up @@ -218,6 +218,7 @@ export function args(rawArgv: string[]): Args {
'integration-version',
'prune-repeated-subdependencies',
'dry-run',
'update-sequentially',
];
for (const dashedArg of argumentsToTransform) {
if (argv[dashedArg]) {
Expand Down
4 changes: 3 additions & 1 deletion src/cli/commands/fix/index.ts
Expand Up @@ -25,6 +25,7 @@ const snykFixFeatureFlag = 'cliSnykFix';
interface FixOptions {
dryRun?: boolean;
quiet?: boolean;
updateSequentially?: boolean;
}
export default async function fix(...args: MethodArgs): Promise<string> {
const { options: rawOptions, paths } = await processCommandArgs<FixOptions>(
Expand All @@ -44,12 +45,13 @@ export default async function fix(...args: MethodArgs): Promise<string> {
const vulnerableResults = results.filter(
(res) => Object.keys(res.testResult.issues).length,
);
const { dryRun, quiet } = options;
const { dryRun, quiet, updateSequentially: sequentialFix } = options;
const { fixSummary, meta, results: resultsByPlugin } = await snykFix.fix(
results,
{
dryRun,
quiet,
sequentialFix,
},
);

Expand Down
1 change: 1 addition & 0 deletions src/lib/types.ts
Expand Up @@ -163,6 +163,7 @@ export type SupportedUserReachableFacingCliArgs =
| 'detection-depth'
| 'docker'
| 'dry-run'
| 'update-sequentially'
| 'fail-on'
| 'file'
| 'gradle-sub-project'
Expand Down

0 comments on commit d820026

Please sign in to comment.