Skip to content

Commit 5b744db

Browse files
committed
Fix unhandled rejection when S3 route used without the optional SDK
response.getLink() invokes s3-service.getSignedUrl in callback style and ignores its returned promise, relying on the callback always firing. The lazy-import refactor could reject getSignedUrl (SDK missing) before the callback ran, turning the ignored promise into an unhandled rejection (which crashes the process under Node's default) instead of the graceful 500 main produced. Wrap getSignedUrl in try/catch so it always resolves via the callback and never rejects — restoring the original contract. Add an e2e guard (s3-no-sdk fixture): an S3 route without @aws-sdk must return a handled 5xx and not crash. Layer 1 now 17/17.
1 parent 52d597a commit 5b744db

4 files changed

Lines changed: 57 additions & 20 deletions

File tree

e2e/fixtures/s3-no-sdk/handler.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
// An S3 route in a project WITHOUT the optional @aws-sdk installed must fail GRACEFULLY
3+
// (handled 5xx), never crash the process with an unhandled rejection. Node's default
4+
// (unhandled rejections terminate the process) means a leak here exits non-zero -> the
5+
// runner sees a non-zero status and fails. The 50ms settle lets any stray rejection surface.
6+
const createAPI = require('lambda-api');
7+
const { readFileSync } = require('fs');
8+
const api = createAPI({ version: 'v1' });
9+
api.get('/ok', (req, res) => res.json({ ok: true }));
10+
api.get('/link', async (req, res) => {
11+
const url = await res.getLink('s3://bucket/key.txt');
12+
return { url };
13+
});
14+
const event = JSON.parse(readFileSync(process.argv[2], 'utf8'));
15+
api
16+
.run(event, {})
17+
.then((r) => setTimeout(() => process.stdout.write(JSON.stringify(r)), 50))
18+
.catch((e) => { process.stderr.write(String(e.stack || e)); process.exit(1); });
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{ "name": "fixture-s3-no-sdk", "private": true }

e2e/run-layer1.mjs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,23 @@ section('esm-import (no optional @aws-sdk installed)');
7070
});
7171
}
7272

73+
// 2b. An S3 route WITHOUT the optional SDK must fail gracefully (handled 5xx), never crash
74+
// the process with an unhandled rejection (regression guard: getSignedUrl must not reject).
75+
section('s3 route without @aws-sdk -> graceful 5xx (no unhandled rejection)');
76+
{
77+
const dir = stageFixture(base, 's3-no-sdk');
78+
check('non-S3 route still 200', () => {
79+
const r = parseResponse(runNode(dir, 'handler.js', writeEvent(dir, 'ok.json', apiGatewayV1('/ok'))), 's3-no-sdk /ok');
80+
assert(r.statusCode === 200, `status ${r.statusCode}`);
81+
});
82+
check('S3 route -> handled 5xx, process did not crash', () => {
83+
const res = runNode(dir, 'handler.js', writeEvent(dir, 'link.json', apiGatewayV1('/link')));
84+
assert(res.status === 0, `process crashed (unhandled rejection?) status=${res.status}\n${res.stderr || ''}`);
85+
const r = JSON.parse(res.stdout);
86+
assert(r.statusCode >= 500 && r.statusCode < 600, `expected 5xx, got ${r.statusCode}`);
87+
});
88+
}
89+
7390
// 3. deep imports via the exports `./lib/*` map
7491
section('deep-import lambda-api/lib/utils (cjs + esm)');
7592
{

src/lib/s3-service.js

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,27 +49,28 @@ export const getSignedUrl = async (
4949
{ Expires, ...params },
5050
callback = () => {}
5151
) => {
52-
const { GetObjectCommand } = await import('@aws-sdk/client-s3');
53-
const { getSignedUrl: awsGetSignedUrl } = await import(
54-
'@aws-sdk/s3-request-presigner'
55-
);
56-
let command;
57-
switch (type) {
58-
case 'getObject':
59-
command = new GetObjectCommand(params);
60-
break;
61-
default:
62-
throw new Error('Invalid command type');
52+
// Callers (response.getLink) use the callback and ignore the returned promise, so this must
53+
// ALWAYS resolve via the callback and never reject — including when the lazy SDK import fails.
54+
try {
55+
const { GetObjectCommand } = await import('@aws-sdk/client-s3');
56+
const { getSignedUrl: awsGetSignedUrl } = await import(
57+
'@aws-sdk/s3-request-presigner'
58+
);
59+
let command;
60+
switch (type) {
61+
case 'getObject':
62+
command = new GetObjectCommand(params);
63+
break;
64+
default:
65+
throw new Error('Invalid command type');
66+
}
67+
const client = await getClient();
68+
const url = await awsGetSignedUrl(client, command, { expiresIn: Expires });
69+
callback(null, url);
70+
return url;
71+
} catch (err) {
72+
callback(err);
6373
}
64-
const client = await getClient();
65-
return awsGetSignedUrl(client, command, { expiresIn: Expires })
66-
.then((url) => {
67-
callback(null, url);
68-
return url;
69-
})
70-
.catch((err) => {
71-
callback(err);
72-
});
7374
};
7475

7576
const service = {

0 commit comments

Comments
 (0)