Skip to content

Commit

Permalink
feat(AWS HTTP API): Drop support for timeout setting
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
`timeout` setting as configured directly for `httpApi` event,  is no longer supported.  Timeout value is now unconditionally resolved from  function timeout setting. It's to guarantee that configured endpoint has necessary room to process function invocation
  • Loading branch information
medikoo committed Sep 10, 2020
1 parent 615b10b commit 1cfd1f2
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 94 deletions.
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
38 changes: 0 additions & 38 deletions lib/plugins/aws/package/compile/events/httpApi/index.test.js
Expand Up @@ -602,42 +602,4 @@ describe('HttpApiEvents', () => {
});
});
});

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

before(() =>
runServerless({
fixture: 'httpApi',
configExt: {
provider: { httpApi: { timeout: 3 } },
functions: {
other: {
events: [
{
httpApi: {
timeout: 20.56,
},
},
],
},
},
},
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 @@ -295,7 +295,6 @@ class AwsProvider {
},
name: { type: 'string' },
payload: { type: 'string' },
timeout: { type: 'number', minimum: 0.05, maximum: 30 },
},
additionalProperties: false,
},
Expand Down

0 comments on commit 1cfd1f2

Please sign in to comment.