Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

Fixing time bug in webhook tests (#383)

Browse Source 
Webhook tests were reusing date in logs, which caused a random failure in tests that checked updatedTime.
This commit is contained in:
jarek 2022-12-22 18:15:06 +01:00 committed by GitHub
parent 5ef591434d
commit 506f61838a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
app/server/lib/Triggers.ts
View File

@ -655,7 +655,6 @@ export class DocTriggers {
private async _sendWebhookWithRetries(id: string, url: string, body: string, size: number, signal: AbortSignal) { private async _sendWebhookWithRetries(id: string, url: string, body: string, size: number, signal: AbortSignal) {
const maxWait = 64; const maxWait = 64;
let wait = 1; let wait = 1;
let now = Date.now();
for (let attempt = 0; attempt < this._maxWebhookAttempts; attempt++) { for (let attempt = 0; attempt < this._maxWebhookAttempts; attempt++) {
if (this._shuttingDown) { if (this._shuttingDown) {
return false; return false;
@ -672,12 +671,11 @@ export class DocTriggers {
}, },
signal, signal,
}); });
now = Date.now();
if (response.status === 200) { if (response.status === 200) {
await this._stats.logBatch(id, 'success', now, { size, httpStatus: 200, error: null, attempts: attempt + 1 }); await this._stats.logBatch(id, 'success', { size, httpStatus: 200, error: null, attempts: attempt + 1 });
return true; return true;
} }
await this._stats.logBatch(id, 'failure', now, { await this._stats.logBatch(id, 'failure', {
httpStatus: response.status, httpStatus: response.status,
error: await response.text(), error: await response.text(),
attempts: attempt + 1, attempts: attempt + 1,
@ -685,7 +683,7 @@ export class DocTriggers {
}); });
this._log(`Webhook responded with non-200 status`, {level: 'warn', status: response.status, attempt}); this._log(`Webhook responded with non-200 status`, {level: 'warn', status: response.status, attempt});
} catch (e) { } catch (e) {
await this._stats.logBatch(id, 'failure', now, { await this._stats.logBatch(id, 'failure', {
httpStatus: null, httpStatus: null,
error: (e.message || 'Unrecognized error during fetch'), error: (e.message || 'Unrecognized error during fetch'),
attempts: attempt + 1, attempts: attempt + 1,
@ -873,7 +871,6 @@ class WebhookStatistics extends PersistedStore<StatsKey> {
public async logBatch( public async logBatch(
id: string, id: string,
status: WebhookBatchStatus, status: WebhookBatchStatus,
now?: number|null,
stats?: { stats?: {
httpStatus?: number|null, httpStatus?: number|null,
error?: string|null, error?: string|null,
@ -881,7 +878,7 @@ class WebhookStatistics extends PersistedStore<StatsKey> {
attempts?: number|null, attempts?: number|null,
} }
) { ) {
now ??= Date.now(); const now = Date.now();
// Update batchStats. // Update batchStats.
const batchStats: [StatsKey, string][] = [ const batchStats: [StatsKey, string][] = [

13
test/server/lib/DocApi.ts
View File

@ -3534,7 +3534,6 @@ function testDocApi() {
// Ok, we can return success now. // Ok, we can return success now.
probeStatus = 200; probeStatus = 200;
controller.abort(); controller.abort();
now = Date.now();
await longFinished.waitAndReset(); await longFinished.waitAndReset();
// After releasing the hook, we are not 100% sure stats are updated, so we will wait a bit. // After releasing the hook, we are not 100% sure stats are updated, so we will wait a bit.
// If we are checking stats while we are holding the hook (in the probe endpoint) it is safe // If we are checking stats while we are holding the hook (in the probe endpoint) it is safe
@ -3545,10 +3544,10 @@ function testDocApi() {
}, 1000, 200); }, 1000, 200);
assert.equal(stats[0].usage?.numWaiting, 0); assert.equal(stats[0].usage?.numWaiting, 0);
assert.equal(stats[0].usage?.status, 'idle'); assert.equal(stats[0].usage?.status, 'idle');
assert.isAbove(stats[0].usage?.updatedTime ?? 0, now); assert.isAtLeast(stats[0].usage?.updatedTime ?? 0, now);
assert.isNull(stats[0].usage?.lastErrorMessage); assert.isNull(stats[0].usage?.lastErrorMessage);
assert.isNull(stats[0].usage?.lastFailureTime); assert.isNull(stats[0].usage?.lastFailureTime);
assert.isAbove(stats[0].usage?.lastSuccessTime ?? 0, now); assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
assert.equal(stats[0].usage?.lastHttpStatus, 200); assert.equal(stats[0].usage?.lastHttpStatus, 200);
assert.deepEqual(stats[0].usage?.lastEventBatch, { assert.deepEqual(stats[0].usage?.lastEventBatch, {
status: 'success', status: 'success',
@ -3575,11 +3574,11 @@ function testDocApi() {
stats = await readStats(docId); stats = await readStats(docId);
assert.equal(stats[0].usage?.numWaiting, 1); assert.equal(stats[0].usage?.numWaiting, 1);
assert.equal(stats[0].usage?.status, 'retrying'); assert.equal(stats[0].usage?.status, 'retrying');
assert.isAbove(stats[0].usage?.updatedTime ?? 0, now); assert.isAtLeast(stats[0].usage?.updatedTime ?? 0, now);
// There was no body in the response yet. // There was no body in the response yet.
assert.isNull(stats[0].usage?.lastErrorMessage); assert.isNull(stats[0].usage?.lastErrorMessage);
// Now we have a failure, and the success was before. // Now we have a failure, and the success was before.
assert.isAbove(stats[0].usage?.lastFailureTime ?? 0, now); assert.isAtLeast(stats[0].usage?.lastFailureTime ?? 0, now);
assert.isBelow(stats[0].usage?.lastSuccessTime ?? 0, now); assert.isBelow(stats[0].usage?.lastSuccessTime ?? 0, now);
assert.equal(stats[0].usage?.lastHttpStatus, 404); assert.equal(stats[0].usage?.lastHttpStatus, 404);
// Batch contains info about last attempt. // Batch contains info about last attempt.
@ -3623,8 +3622,8 @@ function testDocApi() {
assert.equal(stats[0].usage?.status, 'idle'); assert.equal(stats[0].usage?.status, 'idle');
assert.equal(stats[0].usage?.lastHttpStatus, 200); assert.equal(stats[0].usage?.lastHttpStatus, 200);
assert.equal(stats[0].usage?.lastErrorMessage, probeMessage); assert.equal(stats[0].usage?.lastErrorMessage, probeMessage);
assert.isAbove(stats[0].usage?.lastFailureTime ?? 0, now); assert.isAtLeast(stats[0].usage?.lastFailureTime ?? 0, now);
assert.isAbove(stats[0].usage?.lastSuccessTime ?? 0, now); assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
assert.deepEqual(stats[0].usage?.lastEventBatch, { assert.deepEqual(stats[0].usage?.lastEventBatch, {
status: 'success', status: 'success',
attempts: 3, attempts: 3,