Improve Request property defaults (#719)

* Improve Request property defaults * Fix flaky tests
apify · Jun 5, 2020 · cb0bbf4 · cb0bbf4
1 parent 3996577
commit cb0bbf4
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 38 deletions.
diff --git a/src/request.js b/src/request.js
@@ -1,4 +1,3 @@
-import * as _ from 'underscore';
 import * as util from 'util';
 import * as crypto from 'crypto';
 import { checkParamOrThrow } from 'apify-client/build/utils';
@@ -87,16 +86,16 @@ class Request {
         const {
             id,
             url,
-            loadedUrl = null,
+            loadedUrl,
             uniqueKey,
             method = 'GET',
-            payload = null,
+            payload,
             noRetry = false,
             retryCount = 0,
-            errorMessages = null,
+            errorMessages = [],
             headers = {},
             userData = {},
-            handledAt = null,
+            handledAt,
             keepUrlFragment = false,
             useExtendedUniqueKey = false,
         } = options;
@@ -132,12 +131,12 @@ class Request {
         this.headers = JSON.parse(JSON.stringify(headers));
         this.userData = JSON.parse(JSON.stringify(userData));
 
+        this.handledAt = handledAt;
         // Requests received from API will have ISOString dates,
         // but we want to have a Date instance.
-        // eslint-disable-next-line no-nested-ternary
-        this.handledAt = _.isDate(handledAt)
-            ? handledAt
-            : (handledAt ? new Date(handledAt) : null);
+        if (typeof handledAt === 'string') {
+            this.handledAt = new Date(handledAt);
+        }
     }
 
     /**
@@ -183,7 +182,6 @@ class Request {
             message = errorOrMessage.toString();
         }
 
-        if (!this.errorMessages) this.errorMessages = [];
         this.errorMessages.push(message);
     }
 

diff --git a/test/crawlers/basic_crawler.test.js b/test/crawlers/basic_crawler.test.js
@@ -135,10 +135,10 @@ describe('BasicCrawler', () => {
         await basicCrawler.run();
 
         expect(processed['http://example.com/1'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/1'].errorMessages).toBeNull();
+        expect(processed['http://example.com/1'].errorMessages).toEqual([]);
         expect(processed['http://example.com/1'].retryCount).toBe(0);
         expect(processed['http://example.com/3'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/3'].errorMessages).toBeNull();
+        expect(processed['http://example.com/3'].errorMessages).toEqual([]);
         expect(processed['http://example.com/3'].retryCount).toBe(0);
 
         expect(processed['http://example.com/2'].userData.foo).toBeUndefined();
@@ -380,10 +380,10 @@ describe('BasicCrawler', () => {
         await basicCrawler.run();
 
         expect(processed['http://example.com/0'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/0'].errorMessages).toBeNull();
+        expect(processed['http://example.com/0'].errorMessages).toEqual([]);
         expect(processed['http://example.com/0'].retryCount).toBe(0);
         expect(processed['http://example.com/2'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/2'].errorMessages).toBeNull();
+        expect(processed['http://example.com/2'].errorMessages).toEqual([]);
         expect(processed['http://example.com/2'].retryCount).toBe(0);
 
         expect(processed['http://example.com/1'].userData.foo).toBeUndefined();
@@ -497,10 +497,10 @@ describe('BasicCrawler', () => {
         await basicCrawler.run();
 
         expect(processed['http://example.com/1'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/1'].errorMessages).toBeNull();
+        expect(processed['http://example.com/1'].errorMessages).toEqual([]);
         expect(processed['http://example.com/1'].retryCount).toBe(0);
         expect(processed['http://example.com/3'].userData.foo).toBe('bar');
-        expect(processed['http://example.com/3'].errorMessages).toBeNull();
+        expect(processed['http://example.com/3'].errorMessages).toEqual([]);
         expect(processed['http://example.com/3'].retryCount).toBe(0);
 
         expect(processed['http://example.com/2'].userData.foo).toEqual(undefined);

diff --git a/test/events.test.js b/test/events.test.js
@@ -8,13 +8,16 @@ import Apify from '../build';
 
 describe('Apify.events', () => {
     let wss = null;
+    let clock;
     beforeEach(() => {
         wss = new WebSocket.Server({ port: 9099 });
+        clock = sinon.useFakeTimers();
         process.env[ENV_VARS.ACTOR_EVENTS_WS_URL] = 'ws://localhost:9099/someRunId';
         process.env[ENV_VARS.TOKEN] = 'dummy';
     });
     afterEach((done) => {
         wss.close(done);
+        clock.restore();
         delete process.env[ENV_VARS.ACTOR_EVENTS_WS_URL];
         delete process.env[ENV_VARS.TOKEN];
     });
@@ -58,7 +61,9 @@ describe('Apify.events', () => {
         Apify.main(async () => {
             await isWsConnected;
             Apify.events.on('name-1', data => eventsReceived.push(data));
-            await sleep(1000);
+            clock.tick(150);
+            clock.restore();
+            await sleep(10);
         });
 
         // Main will call process.exit() so we must stub it.
@@ -80,10 +85,15 @@ describe('Apify.events', () => {
 
     test('should work without Apify.main()', async () => {
         let wsClosed = false;
+        let finish;
+        const closePromise = new Promise((resolve) => {
+            finish = resolve;
+        });
         const isWsConnected = new Promise((resolve) => {
             wss.on('connection', (ws, req) => {
                 ws.on('close', () => {
                     wsClosed = true;
+                    finish();
                 });
                 resolve(ws);
 
@@ -104,34 +114,28 @@ describe('Apify.events', () => {
         await Apify.initializeEvents();
         await isWsConnected;
         Apify.events.on('name-1', data => eventsReceived.push(data));
-        await sleep(1000);
+        clock.tick(150);
+        clock.restore();
+        await sleep(10);
 
         expect(eventsReceived).toEqual([[1, 2, 3], { foo: 'bar' }]);
 
         expect(wsClosed).toBe(false);
         Apify.stopEvents();
-        await sleep(10); // Here must be short sleep to get following line to later tick
+        await closePromise;
         expect(wsClosed).toBe(true);
     });
 
     test('should send persist state events in regular interval', async () => {
-        const clock = sinon.useFakeTimers();
-        process.env.APIFY_TEST_PERSIST_INTERVAL_MILLIS = 1;
-
-        try {
-            const eventsReceived = [];
-            Apify.events.on(ACTOR_EVENT_NAMES_EX.PERSIST_STATE, data => eventsReceived.push(data));
-            await Apify.initializeEvents();
-            clock.tick(1);
-            clock.tick(1);
-            clock.tick(1);
-            await Apify.stopEvents();
-            const eventCount = eventsReceived.length;
-            expect(eventCount).toBe(3);
-            expect(eventsReceived.length).toEqual(eventCount);
-        } finally {
-            clock.restore();
-            delete process.env.APIFY_TEST_PERSIST_INTERVAL_MILLIS;
-        }
+        const eventsReceived = [];
+        Apify.events.on(ACTOR_EVENT_NAMES_EX.PERSIST_STATE, data => eventsReceived.push(data));
+        await Apify.initializeEvents();
+        clock.tick(60001);
+        clock.tick(60001);
+        clock.tick(60001);
+        await Apify.stopEvents();
+        const eventCount = eventsReceived.length;
+        expect(eventCount).toBe(3);
+        expect(eventsReceived.length).toEqual(eventCount);
     });
 });
diff --git a/test/request.test.js b/test/request.test.js
@@ -62,7 +62,7 @@ describe('Apify.Request', () => {
     test('should allow to push error messages', () => {
         const request = new Apify.Request({ url: 'http://example.com' });
 
-        expect(request.errorMessages).toBe(null);
+        expect(request.errorMessages).toEqual([]);
 
         // Make a circular, unstringifiable object.
         const circularObj = { prop: 1 };