Skip to content

Commit 17ae585

Browse files
committed
handle unsupported http methods
1 parent a56e559 commit 17ae585

File tree

3 files changed

+57
-17
lines changed

3 files changed

+57
-17
lines changed

packages/event-handler/src/rest/BaseRouter.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,22 @@ import type {
1515
RouteOptions,
1616
RouterOptions,
1717
} from '../types/rest.js';
18-
import { HttpVerbs } from './constants.js';
18+
import { HttpErrorCodes, HttpVerbs } from './constants.js';
1919
import {
2020
handlerResultToProxyResult,
2121
proxyEventToWebRequest,
2222
responseToProxyResult,
2323
} from './converters.js';
2424
import { ErrorHandlerRegistry } from './ErrorHandlerRegistry.js';
2525
import {
26+
InternalServerError,
2627
MethodNotAllowedError,
2728
NotFoundError,
2829
ServiceError,
2930
} from './errors.js';
3031
import { Route } from './Route.js';
3132
import { RouteHandlerRegistry } from './RouteHandlerRegistry.js';
32-
import { isAPIGatewayProxyEvent } from './utils.js';
33+
import { isAPIGatewayProxyEvent, isHttpMethod } from './utils.js';
3334

3435
abstract class BaseRouter {
3536
protected context: Record<string, unknown>;
@@ -156,16 +157,27 @@ abstract class BaseRouter {
156157
options?: ResolveOptions
157158
): Promise<APIGatewayProxyResult | undefined> {
158159
if (!isAPIGatewayProxyEvent(event)) {
159-
this.logger.warn(
160+
this.logger.error(
160161
'Received an event that is not compatible with this resolver'
161162
);
162-
return;
163+
throw new InternalServerError();
164+
}
165+
166+
const method = event.requestContext.httpMethod.toUpperCase();
167+
if (!isHttpMethod(method)) {
168+
this.logger.error(`HTTP method ${method} is not supported.`);
169+
// We can't throw a MethodNotAllowedError outside the try block as it
170+
// will be converted to an internal server error by the API Gateway runtime
171+
return {
172+
statusCode: HttpErrorCodes.METHOD_NOT_ALLOWED,
173+
body: '',
174+
};
163175
}
164176

177+
const request = proxyEventToWebRequest(event);
178+
165179
try {
166-
const request = proxyEventToWebRequest(event);
167180
const path = new URL(request.url).pathname as Path;
168-
const method = request.method.toUpperCase() as HttpMethod;
169181

170182
const route = this.routeRegistry.resolve(method, path);
171183

@@ -184,8 +196,9 @@ abstract class BaseRouter {
184196

185197
return await handlerResultToProxyResult(result);
186198
} catch (error) {
199+
this.logger.debug(`There was an error processing the request: ${error}`);
187200
const result = await this.handleError(error as Error, {
188-
request: proxyEventToWebRequest(event),
201+
request,
189202
event,
190203
context,
191204
scope: options?.scope,

packages/event-handler/src/rest/utils.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
import { isRecord, isString } from '@aws-lambda-powertools/commons/typeutils';
22
import type { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda';
3-
import type { CompiledRoute, Path, ValidationResult } from '../types/rest.js';
4-
import { PARAM_PATTERN, SAFE_CHARS, UNSAFE_CHARS } from './constants.js';
3+
import type {
4+
CompiledRoute,
5+
HttpMethod,
6+
Path,
7+
ValidationResult,
8+
} from '../types/rest.js';
9+
import {
10+
HttpVerbs,
11+
PARAM_PATTERN,
12+
SAFE_CHARS,
13+
UNSAFE_CHARS,
14+
} from './constants.js';
515

616
export function compilePath(path: Path): CompiledRoute {
717
const paramNames: string[] = [];
@@ -69,6 +79,10 @@ export const isAPIGatewayProxyEvent = (
6979
);
7080
};
7181

82+
export const isHttpMethod = (method: string): method is HttpMethod => {
83+
return Object.keys(HttpVerbs).includes(method);
84+
};
85+
7286
/**
7387
* Type guard to check if the provided result is an API Gateway Proxy result.
7488
*

packages/event-handler/tests/unit/rest/BaseRouter.test.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,23 @@ describe('Class: BaseRouter', () => {
7979
});
8080
});
8181

82+
it.each([['CONNECT'], ['TRACE']])(
83+
'throws MethodNotAllowedError for %s requests',
84+
async (method) => {
85+
// Prepare
86+
const app = new TestResolver();
87+
88+
// Act & Assess
89+
const result = await app.resolve(
90+
createTestEvent('/test', method),
91+
context
92+
);
93+
94+
expect(result?.statusCode).toBe(HttpErrorCodes.METHOD_NOT_ALLOWED);
95+
expect(result?.body).toEqual('');
96+
}
97+
);
98+
8299
it('accepts multiple HTTP methods', async () => {
83100
// Act
84101
const app = new TestResolver();
@@ -965,18 +982,14 @@ describe('Class: BaseRouter', () => {
965982
});
966983

967984
describe('resolve method', () => {
968-
it('returns undefined and logs warning for non-API Gateway events', async () => {
985+
it('throws an internal server error for non-API Gateway events', async () => {
969986
// Prepare
970987
const app = new TestResolver();
971988
const nonApiGatewayEvent = { Records: [] }; // SQS-like event
972989

973-
// Act
974-
const result = await app.resolve(nonApiGatewayEvent, context);
975-
976-
// Assess
977-
expect(result).toBeUndefined();
978-
expect(console.warn).toHaveBeenCalledWith(
979-
'Received an event that is not compatible with this resolver'
990+
// Act & Assess
991+
expect(app.resolve(nonApiGatewayEvent, context)).rejects.toThrowError(
992+
InternalServerError
980993
);
981994
});
982995

0 commit comments

Comments
 (0)