Skip to content
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

AWS HTTP API: Drop support for timeout setting #8184

Merged
merged 1 commit into from Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 20 additions & 55 deletions lib/plugins/aws/package/compile/events/httpApi/index.js
Expand Up @@ -79,7 +79,6 @@ class HttpApiEvents {
},
method: { type: 'string', regexp: methodPattern.toString() },
path: { type: 'string', regexp: /^(?:\*|\/\S*)$/.toString() },
timeout: { type: 'number', minimum: 0.05, maximum: 30 },
},
required: ['path'],
additionalProperties: false,
Expand Down Expand Up @@ -303,13 +302,6 @@ Object.defineProperties(
})}`;
}

if (userConfig.timeout) {
this.serverless._logDeprecation(
'AWS_HTTP_API_TIMEOUT',
'"provider.httpApi.timeout" is deprecated. ' +
'HTTP API endpoints are configured to follow timeout setting as set for functions.'
);
}
for (const [functionName, functionData] of _.entries(this.serverless.service.functions)) {
const routeTargetData = {
functionName,
Expand All @@ -323,9 +315,8 @@ Object.defineProperties(
let method;
let path;
let authorizer;
let timeout;
if (_.isObject(event.httpApi)) {
({ method, path, authorizer, timeout } = event.httpApi);
({ method, path, authorizer } = event.httpApi);
} else {
const methodPath = String(event.httpApi);
if (methodPath === '*') {
Expand Down Expand Up @@ -415,17 +406,6 @@ Object.defineProperties(
}
if (scopes) routeConfig.authorizationScopes = toSet(scopes);
}
if (!timeout) timeout = userConfig.timeout || null;
if (typeof routeTargetData.timeout !== 'undefined') {
if (routeTargetData.timeout !== timeout) {
throw new this.serverless.classes.Error(
`Inconsistent timeout settings for ${functionName} events`,
'INCONSISTENT_HTTP_API_TIMEOUT'
);
}
} else {
routeTargetData.timeout = timeout;
}
routes.set(routeKey, routeConfig);
if (shouldFillCorsMethods) {
if (event.resolvedMethod === 'ANY') {
Expand All @@ -440,42 +420,27 @@ Object.defineProperties(
if (!hasHttpApiEvents) continue;
const functionTimeout =
Number(functionData.timeout) || Number(this.serverless.service.provider.timeout) || 6;
if (routeTargetData.timeout) {
this.serverless._logDeprecation(
'AWS_HTTP_API_TIMEOUT',
`"functions[].events.httpApi.timeout" is deprecated (found one defined for '${functionName}'s endpoint). ` +
'HTTP API endpoints are configured to follow timeout setting as set for functions.'

if (functionTimeout > 29) {
logWarning(
`Function (${functionName}) timeout setting (${functionTimeout}) is greater than ` +
'maximum allowed timeout for HTTP API endpoint (29s). ' +
'This may introduce a situation where endpoint times out ' +
'for a succesful lambda invocation.'
);
} else if (functionTimeout === 29) {
logWarning(
`Function (${functionName}) timeout setting (${functionTimeout}) may not provide ` +
'enough room to process an HTTP API request (of which timeout is limited to 29s). ' +
'This may introduce a situation where endpoint times out ' +
'for a succesful lambda invocation.'
);
if (functionTimeout >= routeTargetData.timeout) {
logWarning(
`HTTP API endpoint timeout setting (${routeTargetData.timeout}s) is ` +
`lower or equal to function (${functionName}) timeout (${functionTimeout}s). ` +
'This may introduce a situation where endpoint times out ' +
'for a succesful lambda invocation.'
);
}
} else {
if (functionTimeout > 29) {
logWarning(
`Function (${functionName}) timeout setting (${functionTimeout}) is greater than ` +
'maximum allowed timeout for HTTP API endpoint (29s). ' +
'This may introduce a situation where endpoint times out ' +
'for a succesful lambda invocation.'
);
} else if (functionTimeout === 29) {
logWarning(
`Function (${functionName}) timeout setting (${functionTimeout}) may not provide ` +
'enough room to process an HTTP API request (of which timeout is limited to 29s). ' +
'This may introduce a situation where endpoint times out ' +
'for a succesful lambda invocation.'
);
}
// Ensure endpoint has slightly larger timeout than a function,
// It's a margin needed for some side processing time on AWS side.
// Otherwise there's a risk of observing 503 status for successfully resolved invocation
// (which just fit function timeout setting)
routeTargetData.timeout = Math.min(functionTimeout + 0.5, 29);
}
// Ensure endpoint has slightly larger timeout than a function,
// It's a margin needed for some side processing time on AWS side.
// Otherwise there's a risk of observing 503 status for successfully resolved invocation
// (which just fit function timeout setting)
routeTargetData.timeout = Math.min(functionTimeout + 0.5, 29);
}
}),
compileIntegration: d(function(routeTargetData) {
Expand Down
41 changes: 0 additions & 41 deletions lib/plugins/aws/package/compile/events/httpApi/index.test.js
Expand Up @@ -641,45 +641,4 @@ describe('HttpApiEvents', () => {
});
});
});

describe('Timeout', () => {
let cfResources;
let naming;

before(() =>
fixtures
.extend('httpApi', {
provider: { httpApi: { timeout: 3 } },
functions: {
other: {
events: [
{
httpApi: {
timeout: 20.56,
},
},
],
},
},
})
.then(fixturePath =>
runServerless({
cwd: fixturePath,
cliArgs: ['package'],
}).then(({ awsNaming, cfTemplate }) => {
({ Resources: cfResources } = cfTemplate);
naming = awsNaming;
})
)
);

it('Should support timeout set at endpoint', () => {
const resource = cfResources[naming.getHttpApiIntegrationLogicalId('other')];
expect(resource.Properties.TimeoutInMillis).to.equal(20560);
});
it('Should support globally set timeout', () => {
const resource = cfResources[naming.getHttpApiIntegrationLogicalId('foo')];
expect(resource.Properties.TimeoutInMillis).to.equal(3000);
});
});
});
1 change: 0 additions & 1 deletion lib/plugins/aws/provider/awsProvider.js
Expand Up @@ -275,7 +275,6 @@ class AwsProvider {
},
name: { type: 'string' },
payload: { type: 'string' },
timeout: { type: 'number', minimum: 0.05, maximum: 30 },
},
additionalProperties: false,
},
Expand Down