Skip to content

Commit

Permalink
Merge pull request #270 from charlescapps/charlescapps_issues_269_fix…
Browse files Browse the repository at this point in the history
…-visibility-timeout

Issue #269 - fix the call to ChangeVisibilityTimeout to not include elapsed seconds
  • Loading branch information
tomwinnington authored Aug 19, 2021
2 parents 3cafbb7 + d17e194 commit 7efbc26
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "sqs-consumer",
"version": "5.5.0",
"version": "5.5.1",
"description": "Build SQS-based Node applications without the boilerplate",
"main": "dist/index.js",
"types": "dist/index.d.ts",
Expand Down
14 changes: 6 additions & 8 deletions src/consumer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ export class Consumer extends EventEmitter {
let heartbeat;
try {
if (this.heartbeatInterval) {
heartbeat = this.startHeartbeat(async (elapsedSeconds) => {
return this.changeVisabilityTimeout(message, elapsedSeconds + this.visibilityTimeout);
heartbeat = this.startHeartbeat(async () => {
return this.changeVisabilityTimeout(message, this.visibilityTimeout);
});
}
await this.executeHandler(message);
Expand Down Expand Up @@ -338,8 +338,8 @@ export class Consumer extends EventEmitter {
let heartbeat;
try {
if (this.heartbeatInterval) {
heartbeat = this.startHeartbeat(async (elapsedSeconds) => {
return this.changeVisabilityTimeoutBatch(messages, elapsedSeconds + this.visibilityTimeout);
heartbeat = this.startHeartbeat(async () => {
return this.changeVisabilityTimeoutBatch(messages, this.visibilityTimeout);
});
}
await this.executeBatchHandler(messages);
Expand Down Expand Up @@ -405,11 +405,9 @@ export class Consumer extends EventEmitter {
}
}

private startHeartbeat(heartbeatFn: (elapsedSeconds: number) => void): NodeJS.Timeout {
const startTime = Date.now();
private startHeartbeat(heartbeatFn: () => void): NodeJS.Timeout {
return setInterval(() => {
const elapsedSeconds = Math.ceil((Date.now() - startTime) / 1000);
heartbeatFn(elapsedSeconds);
heartbeatFn();
}, this.heartbeatInterval * 1000);
}
}
20 changes: 10 additions & 10 deletions test/consumer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ describe('Consumer', () => {

});

it('extends visibility timeout for long running handler functions', async () => {
it('uses the correct visibility timeout for long running handler functions', async () => {
consumer = new Consumer({
queueUrl: 'some-queue-url',
region: 'some-region',
Expand All @@ -641,17 +641,17 @@ describe('Consumer', () => {
sandbox.assert.calledWith(sqs.changeMessageVisibility, {
QueueUrl: 'some-queue-url',
ReceiptHandle: 'receipt-handle',
VisibilityTimeout: 70
VisibilityTimeout: 40
});
sandbox.assert.calledWith(sqs.changeMessageVisibility, {
QueueUrl: 'some-queue-url',
ReceiptHandle: 'receipt-handle',
VisibilityTimeout: 100
VisibilityTimeout: 40
});
sandbox.assert.calledOnce(clearIntervalSpy);
});

it('extends visibility timeout for long running batch handler functions', async () => {
it('passes in the correct visibility timeout for long running batch handler functions', async () => {
sqs.receiveMessage = stubResolve({
Messages: [
{ MessageId: '1', ReceiptHandle: 'receipt-handle-1', Body: 'body-1' },
Expand All @@ -677,17 +677,17 @@ describe('Consumer', () => {
sandbox.assert.calledWith(sqs.changeMessageVisibilityBatch, {
QueueUrl: 'some-queue-url',
Entries: [
{ Id: '1', ReceiptHandle: 'receipt-handle-1', VisibilityTimeout: 70 },
{ Id: '2', ReceiptHandle: 'receipt-handle-2', VisibilityTimeout: 70 },
{ Id: '3', ReceiptHandle: 'receipt-handle-3', VisibilityTimeout: 70 }
{ Id: '1', ReceiptHandle: 'receipt-handle-1', VisibilityTimeout: 40 },
{ Id: '2', ReceiptHandle: 'receipt-handle-2', VisibilityTimeout: 40 },
{ Id: '3', ReceiptHandle: 'receipt-handle-3', VisibilityTimeout: 40 }
]
});
sandbox.assert.calledWith(sqs.changeMessageVisibilityBatch, {
QueueUrl: 'some-queue-url',
Entries: [
{ Id: '1', ReceiptHandle: 'receipt-handle-1', VisibilityTimeout: 100 },
{ Id: '2', ReceiptHandle: 'receipt-handle-2', VisibilityTimeout: 100 },
{ Id: '3', ReceiptHandle: 'receipt-handle-3', VisibilityTimeout: 100 }
{ Id: '1', ReceiptHandle: 'receipt-handle-1', VisibilityTimeout: 40 },
{ Id: '2', ReceiptHandle: 'receipt-handle-2', VisibilityTimeout: 40 },
{ Id: '3', ReceiptHandle: 'receipt-handle-3', VisibilityTimeout: 40 }
]
});
sandbox.assert.calledOnce(clearIntervalSpy);
Expand Down

0 comments on commit 7efbc26

Please sign in to comment.