Skip to content

Commit 3aa51e2

Browse files
committed
refactor: drop orphaned sync loader chain; guard dist against stale artifacts
Two follow-ups from PR #784 review: 1. Remove `loadSync`, `_loadUserAppSync`, `_tryRequireSync`. Deleting `src/handler.cjs` in the previous commit removed the only consumers of the synchronous loader chain. `loadSync` was never re-exported from the public `src/index.ts` entry point, no tests reference it, and the only internal import (`src/handler.mjs`) uses the async `load()`. The three functions and the `loadSync` re-export in `src/runtime/index.ts` are now dead code, so drop them. 2. Defensive cleanup in `scripts/update_dist_version.sh`: explicitly `rm -f dist/handler.js dist/handler.cjs` before copying handler.mjs. `tsc` does not clean stale files in `dist/` between incremental builds, so a leftover `handler.js` from a prior build of an earlier ref could resurface and silently re-introduce the resolver bug this PR is fixing. The publish scripts already do `rm -rf ./dist` before `yarn build`, so this is purely a guard for local dev builds.
1 parent ee53944 commit 3aa51e2

3 files changed

Lines changed: 6 additions & 106 deletions

File tree

scripts/update_dist_version.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ echo "Copying handler files"
2020
# `dist/handler.handler` to handler.mjs (it falls through `.js` -> `.mjs`),
2121
# and handler.mjs's async `load()` handles both CJS and ESM user modules, so
2222
# a separate `.js` / `.cjs` variant is no longer needed.
23+
#
24+
# Remove any stale `dist/handler.js` / `dist/handler.cjs` left over from prior
25+
# builds — tsc doesn't clean dist between incremental compiles, and shipping
26+
# either of those files would re-introduce the resolver bug this PR fixes.
27+
rm -f dist/handler.js dist/handler.cjs
2328
cp src/handler.mjs dist/
2429
cp src/init.js dist/init.js
2530
cp src/runtime/module_importer.js dist/runtime/

src/runtime/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
export { load, loadSync } from "./user-function";
1+
export { load } from "./user-function";
22
export { subscribeToDC, getTraceTree, clearTraceTree, RequireNode } from "./require-tracer"

src/runtime/user-function.ts

Lines changed: 0 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -156,45 +156,6 @@ async function _tryRequire(appRoot: string, moduleRoot: string, module: string):
156156
return require(nodeStylePath);
157157
}
158158

159-
/**
160-
* Attempt to load the user's module.
161-
* Attempts to directly resolve the module relative to the application root,
162-
* then falls back to the more general require().
163-
*/
164-
function _tryRequireSync(appRoot: string, moduleRoot: string, module: string): Promise<any> {
165-
const lambdaStylePath = path.resolve(appRoot, moduleRoot, module);
166-
// Extensionless files are loaded via require.
167-
const extensionless = _tryRequireFile(lambdaStylePath);
168-
if (extensionless) {
169-
return extensionless;
170-
}
171-
// If package.json type != module, .js files are loaded via require.
172-
const pjHasModule = _hasPackageJsonTypeModule(lambdaStylePath);
173-
if (!pjHasModule) {
174-
const loaded = _tryRequireFile(lambdaStylePath, ".js");
175-
if (loaded) {
176-
return loaded;
177-
}
178-
}
179-
// If still not loaded, try .js, .mjs, and .cjs in that order.
180-
// Files ending with .js are loaded as ES modules when the nearest parent package.json
181-
// file contains a top-level field "type" with a value of "module".
182-
// https://nodejs.org/api/packages.html#packages_type
183-
const loaded = _tryRequireFile(lambdaStylePath, ".cjs");
184-
if (loaded) {
185-
return loaded;
186-
}
187-
// Why not just require(module)?
188-
// Because require() is relative to __dirname, not process.cwd(). And the
189-
// runtime implementation is not located in /var/task
190-
// This won't work (yet) for esModules as import.meta.resolve is still experimental
191-
// See: https://nodejs.org/api/esm.html#esm_import_meta_resolve_specifier_parent
192-
const nodeStylePath = require.resolve(module, {
193-
paths: [appRoot, moduleRoot],
194-
});
195-
return require(nodeStylePath);
196-
}
197-
198159
/**
199160
* Load the user's application or throw a descriptive error.
200161
* @throws Runtime errors in two cases
@@ -221,26 +182,6 @@ async function _loadUserApp(
221182
}
222183
}
223184

224-
function _loadUserAppSync(
225-
appRoot: string,
226-
moduleRoot: string,
227-
module: string
228-
): Promise<any> {
229-
try {
230-
return _tryRequireSync(appRoot, moduleRoot, module);
231-
} catch (e) {
232-
if (e instanceof SyntaxError) {
233-
throw new UserCodeSyntaxError(<any>e);
234-
// @ts-ignore
235-
} else if (e.code !== undefined && e.code === "MODULE_NOT_FOUND") {
236-
// @ts-ignore
237-
throw new ImportModuleError(e);
238-
} else {
239-
throw e;
240-
}
241-
}
242-
}
243-
244185
function _throwIfInvalidHandler(fullHandlerString: string): void {
245186
if (fullHandlerString.includes(RELATIVE_PATH_SUBSTRING)) {
246187
throw new MalformedHandlerName(
@@ -294,49 +235,3 @@ export const load = async function (
294235

295236
return handlerFunc;
296237
};
297-
298-
/**
299-
* Load the user's function with the approot and the handler string.
300-
* @param appRoot {string}
301-
* The path to the application root.
302-
* @param handlerString {string}
303-
* The user-provided handler function in the form 'module.function'.
304-
* @return userFuction {function}
305-
* The user's handler function. This function will be passed the event body,
306-
* the context object, and the callback function.
307-
* @throws In five cases:-
308-
* 1 - if the handler string is incorrectly formatted an error is thrown
309-
* 2 - if the module referenced by the handler cannot be loaded
310-
* 3 - if the function in the handler does not exist in the module
311-
* 4 - if a property with the same name, but isn't a function, exists on the
312-
* module
313-
* 5 - the handler includes illegal character sequences (like relative paths
314-
* for traversing up the filesystem '..')
315-
* Errors for scenarios known by the runtime, will be wrapped by Runtime.* errors.
316-
*/
317-
export const loadSync = function (
318-
appRoot: string,
319-
fullHandlerString: string
320-
) {
321-
_throwIfInvalidHandler(fullHandlerString);
322-
323-
const [moduleRoot, moduleAndHandler] = _moduleRootAndHandler(
324-
fullHandlerString
325-
);
326-
const [module, handlerPath] = _splitHandlerString(moduleAndHandler);
327-
328-
const userApp = _loadUserAppSync(appRoot, moduleRoot, module);
329-
const handlerFunc = _resolveHandler(userApp, handlerPath);
330-
331-
if (!handlerFunc) {
332-
throw new HandlerNotFound(
333-
`${fullHandlerString} is undefined or not exported`
334-
);
335-
}
336-
337-
if (typeof handlerFunc !== "function") {
338-
throw new HandlerNotFound(`${fullHandlerString} is not a function`);
339-
}
340-
341-
return handlerFunc;
342-
};

0 commit comments

Comments
 (0)