-
Notifications
You must be signed in to change notification settings - Fork 175
feat(event-handler): added compress middleware for the REST API event handler #4495
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@dreamorosi I've addressed the feedback and accepted the regex issue in Sonar |
packages/event-handler/tests/unit/rest/middleware/compress.test.ts
Outdated
Show resolved
Hide resolved
Code duplication is quite high, I suspect because of tests. Can we do anything to bring it down? Also, and I suspect the answer is no, but is there any way to break down that regex into smaller chunks? Not just because Sonar complains (which I'd like to avoid if possible) but also because of complexity. Can we, for example, make an educated guess about a subset of content types that are more common and put these in their own smaller (and faster?) regex, and then fall back to the second (or third) if the previous fail? Does this make sense? |
Yes, I'll try to optimize the tests
Yeah, makes sense. I'll see if I can break it down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've built this branch (npm run build -w pacakges/event-handler
), packed it (npm pack -w packages/event-handler && git restore packages/event-handler/package.json
), and installed the tarball in another dummy project created using the triaging template.
My function looks like this:
import { Router } from '@aws-lambda-powertools/event-handler/experimental-rest';
import { compress } from '@aws-lambda-powertools/event-handler/experimental-rest/middleware';
import { Logger } from '@aws-lambda-powertools/logger';
import type { Context } from 'aws-lambda';
const logger = new Logger({ serviceName: 'pong-service' });
const app = new Router();
app.use(async (_, reqCtx, next) => {
logger.info('Request received', {
headers: reqCtx.event.headers,
});
await next();
});
app.use(compress());
app.get('/ping', async () => {
return { message: 'pong' };
});
export const handler = async (event: unknown, context: Context) =>
app.resolve(event, context);
Click here to see CDK Stack
import { CfnOutput, RemovalPolicy, Stack, type StackProps } from 'aws-cdk-lib';
import { RestApi, LambdaIntegration } from 'aws-cdk-lib/aws-apigateway';
import { Runtime } from 'aws-cdk-lib/aws-lambda';
import { NodejsFunction, OutputFormat } from 'aws-cdk-lib/aws-lambda-nodejs';
import { LogGroup, RetentionDays } from 'aws-cdk-lib/aws-logs';
import type { Construct } from 'constructs';
export class TriageStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);
const fnName = 'PongFn';
const logGroup = new LogGroup(this, 'MyLogGroup', {
logGroupName: `/aws/lambda/${fnName}`,
removalPolicy: RemovalPolicy.DESTROY,
retention: RetentionDays.ONE_DAY,
});
const fn = new NodejsFunction(this, 'MyFunction', {
functionName: fnName,
logGroup,
runtime: Runtime.NODEJS_22_X,
entry: './src/index.ts',
handler: 'handler',
bundling: {
minify: true,
mainFields: ['module', 'main'],
sourceMap: true,
format: OutputFormat.ESM,
},
environment: {
NODE_OPTIONS: '--enable-source-maps',
},
});
const api = new RestApi(this, 'TriageApi', {
restApiName: 'TriageApi',
deployOptions: {
stageName: 'prod',
},
});
const proxyIntegration = new LambdaIntegration(fn, {
proxy: true,
});
// Catch-all ANY method on root and proxy resource
api.root.addMethod('ANY', proxyIntegration);
const proxyResource = api.root.addResource('{proxy+}');
proxyResource.addMethod('ANY', proxyIntegration);
new CfnOutput(this, 'ApiUrl', {
value: api.url,
});
new CfnOutput(this, 'FunctionArn', {
value: fn.functionArn,
});
}
}
If I make a request, the response that I get back looks like this: �����V�M-.NLOU�R*��KW���bk�
and contains the content-encoding: gzip
header.
I tried with curl
, httpie
, and Insomnia both by turning on and off raw mode in all of them.
I'm not sure if I'm doing something wrong or I'm missing something on the infra side. Can you look into it?
@dreamorosi Thanks for looking into this. I will see if there are any other edge cases we need to be aware of. I realized that the actual compressed response hasn't been tested in the code as well so missed this. I'll add the tests for that as well. |
Yes, the |
Ok, thanks. I think we should address that in a separate issue that possibly has its own spec with requirements and all. For the sake of this PR, let's just make sure that when the |
@dreamorosi @svozza I've refactored the code a bit now. It's now using the I've tried testing it out, I get the response already decompressed I think but the ![]() ![]() |
I think that's expected and the result of the content being compressed or not - I guess the only check would be to make sure it matches the expected value in the two cases. |
The responses did match in both the cases |
…powertools-lambda-typescript into feat/compress-middleware
|
Summary
This PR adds a built in compress middleware which can be used along with the event handler for providing an easy way to compress response bodies to reduce payload size and improve performance.
Changes
middleware
directory in the rest directory to contain the built-in middlewaresIssue number: closes #4474
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.