Skip to content

Commit

Permalink
fix: stager fails when the file to download doesn't exist (#1321)
Browse files Browse the repository at this point in the history
Sometimes, the stager function receives a message to download a file
that should exist but doesn' t. If we throw an error, the message will
be sent to the DLQ, to be processed again in a re-drive. But given that
this file will probably never be available, the re-drives will also
fail, and we'll never be able to get rid of the message in the DLQ.
Instead, we ignore this version by returning silently.

NPM seems to occasionally drop tarballs on the floor when a new package
version is released, which you can notice on the download stats because
the package has been downloaded `0` times. So far we've seen that happen
for:

*
[@pepperize/cdk-vpc@0.0.785](https://www.npmjs.com/package/@pepperize/cdk-vpc/v/0.0.785?activeTab=versions)
*
[@aws-sdk/credential-provider-http@3.422.0](https://www.npmjs.com/package/@aws-sdk/credential-provider-http/v/3.422.0?activeTab=versions)

----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*

---------

Signed-off-by: github-actions <github-actions@github.com>
Co-authored-by: github-actions <github-actions@github.com>
  • Loading branch information
otaviomacedo and github-actions committed Sep 29, 2023
1 parent d790bf1 commit 7107eaf
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 12 deletions.
4 changes: 4 additions & 0 deletions .projen/deps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .projen/tasks.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const project = new CdklabsConstructLibrary({
'tar-stream',
'uuid',
'yaml',
'nock',
'normalize-registry-metadata',
'@octokit/rest',
'markdown-it',
Expand Down
1 change: 1 addition & 0 deletions package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions src/__tests__/__snapshots__/construct-hub.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Context } from 'aws-lambda';
import * as AWS from 'aws-sdk';
import * as AWSMock from 'aws-sdk-mock';
import * as nock from 'nock';
import {
ENV_DENY_LIST_BUCKET_NAME,
ENV_DENY_LIST_OBJECT_KEY,
} from '../../../backend/deny-list/constants';
import {
handler,
PackageVersion,
} from '../../../package-sources/npmjs/stage-and-notify.lambda';

const MOCK_DENY_LIST_BUCKET = 'deny-list-bucket-name';
const MOCK_DENY_LIST_OBJECT = 'my-deny-list.json';

type Response<T> = (err: AWS.AWSError | null, data?: T) => void;

beforeEach(() => {
process.env.BUCKET_NAME = 'foo';
process.env.QUEUE_URL = 'bar';
process.env[ENV_DENY_LIST_BUCKET_NAME] = MOCK_DENY_LIST_BUCKET;
process.env[ENV_DENY_LIST_OBJECT_KEY] = MOCK_DENY_LIST_OBJECT;
});

afterEach(() => {
process.env.BUCKET_NAME = undefined;
process.env.QUEUE_URL = undefined;
delete process.env[ENV_DENY_LIST_BUCKET_NAME];
delete process.env[ENV_DENY_LIST_OBJECT_KEY];
});

test('ignores 404', async () => {
const basePath = 'https://registry.npmjs.org';
const uri = '/@pepperize/cdk-vpc/-/cdk-vpc-0.0.785.tgz';

AWSMock.mock(
'S3',
'getObject',
(_req: AWS.S3.GetObjectRequest, cb: Response<AWS.S3.GetObjectOutput>) => {
cb(null, { Body: JSON.stringify({}) });
}
);

nock(basePath).get(uri).reply(404);

const event: PackageVersion = {
tarballUrl: `${basePath}${uri}`,
integrity: '09d37ec93c5518bf4842ac8e381a5c06452500e5',
modified: '2023-09-22T15:48:10.381Z',
name: '@pepper/cdk-vpc',
seq: '26437963',
version: '0.0.785',
};

const context: Context = {} as any;

await expect(handler(event, context)).resolves.toBe(undefined);
});
27 changes: 23 additions & 4 deletions src/package-sources/npmjs/stage-and-notify.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { s3, sqs } from '../../backend/shared/aws.lambda-shared';
import { requireEnv } from '../../backend/shared/env.lambda-shared';
import { integrity } from '../../backend/shared/integrity.lambda-shared';

class HttpNotFoundError extends Error {}

/**
* This function is invoked by the `npm-js-follower.lambda` with a `PackageVersion` object, or by
* an SQS trigger feeding from this function's Dead-Letter Queue (for re-trying purposes).
Expand Down Expand Up @@ -38,9 +40,23 @@ export async function handler(
return;
}

// Download the tarball
console.log(`Downloading tarball from URL: ${event.tarballUrl}`);
const tarball = await httpGet(event.tarballUrl);
let tarball: Buffer = Buffer.from('');
try {
// Download the tarball
console.log(`Downloading tarball from URL: ${event.tarballUrl}`);
tarball = await httpGet(event.tarballUrl);
} catch (e) {
if (e instanceof HttpNotFoundError) {
// We received a message to download a file that should exist but doesn't.
// If we throw an error, the message will be sent to the DLQ, to be processed
// again in a re-drive. But given that this file will probably never be
// available, the re-drives will also fail, and we'll never be able to get rid
// of the message in the DLQ. Instead, we ignore this version by returning silently.
return;
} else {
throw e;
}
}

// Store the tarball into the staging bucket
// - infos.dist.tarball => https://registry.npmjs.org/<@scope>/<name>/-/<name>-<version>.tgz
Expand Down Expand Up @@ -135,7 +151,10 @@ export interface PackageVersion {
function httpGet(url: string) {
return new Promise<Buffer>((ok, ko) => {
https.get(url, (response) => {
if (response.statusCode !== 200) {
console.log(response.statusCode);
if (response.statusCode === 404) {
ko(new HttpNotFoundError());
} else if (response.statusCode !== 200) {
ko(
new Error(
`Unsuccessful GET: ${response.statusCode} - ${response.statusMessage}`
Expand Down
15 changes: 15 additions & 0 deletions yarn.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7107eaf

Please sign in to comment.