Handling events not sent to Adobe Analytics because of internet unavailability by SnchitGrover · Pull Request #14259 · adobe/brackets

SnchitGrover · 2018-04-25T19:34:01Z

No description provided.

…ilability

SnchitGrover · 2018-04-25T19:34:18Z

swmitra · 2018-04-25T20:02:28Z

vickramdhawal · 2018-04-26T03:29:28Z

+
+        unsentEventFileLocation.exists(function (err, exists) {
+            if (err) {
+                console.log("absdjasd");


Log appropriate message. Also if an error occurs the eventParams can still be sent to for logging since internet is available.

vickramdhawal · 2018-04-26T03:32:09Z

+            } else {
+                if(exists) {
+                    FileUtils.readAsText(unsentEventFileLocation).done(function (content) {
+                        FileUtils.writeText(unsentEventFileLocation, "", true).done(function () {


The log file should be cleared only if the data is successfully uploaded.

If I clear the log file as soon as I successfully read it and then if there is any failure I can write those back to log file?

you will have to implement a new shell api readAndClearFile to achieve this else there will always be a synchronisation problem.

vickramdhawal · 2018-04-26T03:32:56Z

+                        sendAnalyticsDataToServer(analyticsData, eventParams).done(function () {
+                            result.resolve();
+                        }).fail(function (err) {
+                            result.reject();


if upload fails, log the data to the unsentEventFileLocation

vickramdhawal · 2018-04-26T03:33:45Z

+                            result.reject();
+                        });
+                    }).fail(function (err) {
+                        result.reject();


If file read fails try to send the current data to the server else this will also be lost.

vickramdhawal · 2018-04-26T03:35:15Z

+    function sendAnalyticsDataToServer (analyticsData, eventParams) {
+        var result = new $.Deferred();
+
+        analyticsData.push(getAnalyticsData(eventParams));


get a single parameter of analyticsData for this method. Push the eventParams before invoking this method.

I am doing this to maintain the order, if I don't do this user action to "cancel" "auto-update" would have a timestamp less than "auto-update" "render" event.

vickramdhawal · 2018-04-26T03:36:23Z

+
+    function internetAvailableSendAllEvents(unsentEventFileLocation, eventParams) {
+        var result = new $.Deferred();
+        var analyticsData = [];


populate the analyticsData with eventParams here itself. It will just be more consistent. This may not be desired if the ordering of events in the array makes a difference.

We should try to maintain the event order.

vickramdhawal · 2018-04-26T03:37:20Z

+                if(exists) {
+                    FileUtils.readAsText(unsentEventFileLocation).done(function (content) {
+                        FileUtils.writeText(unsentEventFileLocation, "", true).done(function () {
+                            result.resolve(true);


what does this "true" signify ?

vickramdhawal · 2018-04-26T03:38:03Z

+                            }
+                        });
+                        sendAnalyticsDataToServer(analyticsData, eventParams).done(function () {
+                            result.resolve();


with the current code the result promise will get resolved twice. once when the files is cleared and once here once the data is uploaded.

vickramdhawal · 2018-04-26T03:38:52Z

+
+        unsentEventFileLocation.exists(function (err, fileExists) {
+            if (err) {
+                result.reject();


log the error for debugging purposes.

vickramdhawal · 2018-04-26T03:43:50Z

                PreferencesManager.setViewState("nextHealthDataSendTime", currentTime + ONE_DAY);
                sendHealthDataToServer().always(function() {
-                    sendAnalyticsDataToServer()
+                    sendAnalyticsDataToServerIfInternetAvailableElseLogtoLocalFile()


simply call it sendAnalyticsData or sendOrSaveAnalyticsData if you feel that information is relevant. The implementation is not open for the client so simply sendAnalyticsData should also be sufficient.

vickramdhawal · 2018-04-26T04:11:44Z

@SnchitGrover You will also need to sign the CLA before the changes can be merged. http://dev.brackets.io/brackets-contributor-license-agreement.html

vickramdhawal · 2018-04-26T05:42:38Z

+            } else {
+                if(exists) {
+                    FileUtils.readAsText(unsentEventFileLocation).done(function (content) {
+                        FileUtils.writeText(unsentEventFileLocation, "", true).done(function () {


you will have to implement a new shell api readAndClearFile to achieve this else there will always be a synchronisation problem.

vickramdhawal · 2018-04-26T08:20:43Z

+            } else {
+                if(exists) {
+                    FileUtils.readAsText(unsentEventFileLocation).done(function (content) {
+                        FileUtils.writeText(unsentEventFileLocation, "", true);


issue of multiple reads is still not resolved.

vickramdhawal · 2018-04-26T08:21:09Z

+        var unsentEventFileLocation = FileSystem.getFileForPath(brackets.app.getApplicationSupportDirectory() + "/unsentEventFile.txt");
+
+        if(window.navigator.onLine){
+            internetAvailableSendAllEvents(unsentEventFileLocation, eventParams).done(function() {


simply call it sendAllEvents

vickramdhawal · 2018-04-26T08:21:25Z

+                result.resolve();
+            });
+        }else {
+            internetUnavailableSaveEventsToDisk(unsentEventFileLocation, eventParams).done(function() {


Simply call it saveEventToDisk.

vickramdhawal · 2018-04-26T08:22:18Z

+                sendAnalyticsDataToServer(unsentEventFileLocation, analyticsData, eventParams).done(function () {
+                    result.resolve();
+                }).fail(function (err) {
+                    console.error("Unable to send file to server");


in this scenario file is not being sent. please correct the log. also, result.reject() should be added.

result.reject() ?

vickramdhawal · 2018-04-26T08:22:28Z

+        unsentEventFileLocation.exists(function (err, exists) {
+            if (err) {
+                // logging the error but will still send the current eventParams which needs to be logged
+                console.error("Error while checking if the error log file exits or not");


nit: exists.

vickramdhawal · 2018-04-26T08:25:01Z

+                        sendAnalyticsDataToServer(unsentEventFileLocation, analyticsData, eventParams).done(function () {
+                            result.resolve();
+                        }).fail(function (err) {
+                            console.error("Unable to send file to server");


if the data is not uploaded to the server it should saved back on the disk.

result.reject() ?

vickramdhawal · 2018-04-26T08:39:29Z

+                    sendAnalyticsDataToServer(unsentEventFileLocation, analyticsData, eventParams).done(function () {
+                        result.resolve();
+                    }).fail(function (err) {
+                        console.error("Unable to send file to server");


in this case file is not being sent to the server.

result.reject() ?

vickramdhawal · 2018-04-26T08:40:40Z

+                            console.error("Unable to send file to server");
+                        });
+                    }).fail(function (err) {
+                        // If reading the file fails try to send currentEventParams to server


add a log here that file read failed.

vickramdhawal · 2018-04-26T08:40:54Z

+                        sendAnalyticsDataToServer(unsentEventFileLocation, analyticsData, eventParams).done(function () {
+                            result.resolve();
+                        }).fail(function (err) {
+                            console.error("Unable to send file to server");


file is not being sent to the server here. please correct the log.

result.reject() ?

vickramdhawal · 2018-04-26T08:42:51Z

+            }).fail(function (jqXHR, status, errorThrown) {
+                // incase request to send data to server fails write the events back to disk
+                analyticsData.forEach(function(event) {
+                    internetUnavailableSaveEventsToDisk(unsentEventFileLocation, eventParams).done(function() {


simply call the method saveEventsToDisk

vickramdhawal · 2018-04-26T09:20:02Z

there are linter errors. please fix those as well.

SnchitGrover · 2018-05-02T12:25:48Z

@vickramdhawal @swmitra

…handler Snchit adobe analytics local handler

SnchitGrover · 2019-03-19T11:48:16Z

Hi @adobe/open-source-office. I am an Adobe active employee. Please take care of the CLA check.

shubhsnov · 2019-03-20T14:07:03Z

@SnchitGrover You can see this to get rid of the CLA check #14606 (comment)

sngrover added 2 commits April 26, 2018 01:00

Handling events not sent to Adobe Analytics because of internet unava…

afd031d

…ilability

Beautify

c3b5a16

SnchitGrover changed the title ~~Handling events not sent to Adobe Analytics because of internet unavailability~~ <WIP Do not Merge>Handling events not sent to Adobe Analytics because of internet unavailability Apr 25, 2018

changing data type of data stored in eventsNotSentFile from csv to JSON

8d8f5a9

vickramdhawal reviewed Apr 26, 2018

View reviewed changes

vickramdhawal suggested changes Apr 26, 2018

View reviewed changes

Addressing review comments

6a7f263

vickramdhawal suggested changes Apr 26, 2018

View reviewed changes

Addressing review comments

74f571c

sngrover added 4 commits May 1, 2018 00:57

New architecture

ad4462c

Changes

6a0d409

Adding comments for Functions

22cbfad

removing unnecessary package.json

89be952

SnchitGrover changed the title ~~<WIP Do not Merge>Handling events not sent to Adobe Analytics because of internet unavailability~~ Handling events not sent to Adobe Analytics because of internet unavailability May 2, 2018

SnchitGrover closed this May 30, 2018

SnchitGrover force-pushed the master branch from 89be952 to 04c4bb3 Compare May 30, 2018 09:49

Merge pull request #1 from SnchitGrover/snchit_adobe_analytics_local_…

d6d83a9

…handler Snchit adobe analytics local handler

SnchitGrover reopened this Mar 19, 2019

Conversation

SnchitGrover commented Apr 25, 2018

Uh oh!

SnchitGrover commented Apr 25, 2018

Uh oh!

swmitra commented Apr 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vickramdhawal Apr 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vickramdhawal commented Apr 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vickramdhawal commented Apr 26, 2018

Uh oh!

SnchitGrover commented May 2, 2018

Uh oh!

SnchitGrover commented Mar 19, 2019

Uh oh!

shubhsnov commented Mar 20, 2019

Uh oh!

Reviewers

Assignees

Labels

vickramdhawal Apr 26, 2018 •

edited

Loading