CottageLabs / LanternPM

Lantern meta repository for product management
1 stars 0 forks source link

Sometimes jobs do not start processing [IE-related] #79

Closed emanuil-tolev closed 8 years ago

emanuil-tolev commented 8 years ago

Possibly related to IE being used to submit the job

Live job, has not moved: https://compliance.cottagelabs.com/#rnrEiitxi7N9ZbrQ9 REPORTEDLY same data was submitted in another live job, worked this time: https://compliance.cottagelabs.com/#gya3PMp58v7wBPLCK

markmacgillivray commented 8 years ago

First job has no items in the list. Was second job also submitted in IE or different browser? Testing in same IE version will be required to see if IE is causing an intermittent error when reading the uploaded sheet.

emanuil-tolev commented 8 years ago

First job has no items in the list. Was second job also submitted in IE or different browser?

The same sheet was reportedly submitted (the one you can download as original from https://compliance.cottagelabs.com/#gya3PMp58v7wBPLCK ) in both cases. The first job is IE, the second job is Chrome, both likely on Windows. I've asked about browser details.

markmacgillivray commented 8 years ago

What version of IE?

http://caniuse.com/#feat=filereader

On Wed, Jul 13, 2016 at 6:55 PM, Emanuil Tolev notifications@github.com wrote:

First job has no items in the list. Was second job also submitted in IE or different browser?

The same sheet was reportedly submitted (the one you can download as original from https://compliance.cottagelabs.com/#gya3PMp58v7wBPLCK ) in both cases. The first job is IE, the second job is Chrome, both likely on Windows. I've asked about browser details.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CottageLabs/LanternPM/issues/79#issuecomment-232435429, or mute the thread https://github.com/notifications/unsubscribe/AAuXCNyeAWKFXHMh0nPbaVIfiAOe0uI6ks5qVSajgaJpZM4JJlCk .

emanuil-tolev commented 8 years ago

What version of IE?

I'm trying to find out.

http://caniuse.com/#feat=filereader

That chart makes it look like a person needs at least IE 10 to use the tool, since IE 8 & 9 do not support this API.

Win 7 / IE9 returns

You must provide at least an ID or a file with at least one record, and if not logged in an email address, in order to submit. Please provide more information and try again.

Win 7 / IE10 seems to get a job stuck at 0%, as described by Wellcome. See https://compliance.cottagelabs.com/#yf8frw364uSuCykXq . I get this from the API endpoint describing the job ( https://api.cottagelabs.com/service/lantern/yf8frw364uSuCykXq ):

{ "status": "success", "data": { "_id": "yf8frw364uSuCykXq", "email": "emanuil.tolev@gmail.com", "refresh": 1, "name": "test_et.csv", "list": [], "createdAt": 1468440473298, "progress": { "progress": null, "name": "test_et.csv", "email": "emanuil.tolev@gmail.com", "_id": "yf8frw364uSuCykXq" } } }

List is empty, so no IDs were taken, yet there also wasn't an error warning the user, like with IE9.

Win 7 / IE 10 produces the same result I'm afraid - 0% job with an empty list attribute. Job: https://compliance.cottagelabs.com/#4Ra3aHu4fYwgfujfE . API info call https://api.cottagelabs.com/service/lantern/4Ra3aHu4fYwgfujfE .

Win 8.1 / IE 11, same 0% deal. Job: https://compliance.cottagelabs.com/#3dKgrvMnikCKcWpmF . API info: https://api.cottagelabs.com/service/lantern/3dKgrvMnikCKcWpmF .

Win 10 / Edge 13 is the first one that seems to work.

I'm not sure this can be fixed or that it is even worth trying to display a warning on upload. Correct me if I'm wrong, but it sounds like a pretty important piece of functionality (reading the file) just doesn't work in IE. At most it seems like we can display a warning to IE users that the site won't work, and to please use Edge or another browser to access it.

emanuil-tolev commented 8 years ago

I do wonder if this could be a pain for any institutional customers ... generally they both a/ have IE that they cannot upgrade and b/ can rarely install software. non-Edge IE now has a miniscule world market share, but probably not quite so small in academia.

emanuil-tolev commented 8 years ago

Decided to support IE if possible. Possibly look at shims and alternatives. I found at least one (though older) https://github.com/Jahdrien/FileReader .

markmacgillivray commented 8 years ago

http://stackoverflow.com/questions/5570871/html5-filereader-alternative

https://social.msdn.microsoft.com/Forums/ie/en-US/25e336eb-8f92-4ee8-8635-4b5c20881ebf/alternative-for-file-api-in-internet-explorer?forum=iewebdevelopment

emanuil-tolev commented 8 years ago

A bit more feedback on this IE issue:

Just to note it still doesn’t work with IE, though it doesn’t work in a different way to previously. Originally it would stick at 0%. It now goes immediately to 100% but when I download the spreadsheet it is blank apart from the column headings. I tested this with the Compliance - analysis_Aug 2015_PubMed spreadsheet.

markmacgillivray commented 8 years ago

That would make sense - the first time the job is created but empty so never completes, and since then I have put in a fix so that jobs that are empty are completed straight away, because essentially they are done. I noted in the code that instead we could deny jobs with nothing in them, but technically, that is a valid job, so I did not do that for now. So I don't think the problem is any worse - behaviour has just changed because of recent updates.

emanuil-tolev commented 8 years ago

Yeah it's the underlying problem persists without change. Just making sure we've seen all the feedback.

emanuil-tolev commented 8 years ago

Adding this to Wellcome milestone since it does not need to be fixed quickly, but it does need to be fixed before finishing the project.

markmacgillivray commented 8 years ago

Does it? So they do want this before signing off? That kind of implies that it is very relevant to the milestone...

On 4 Aug 2016 16:34, "Emanuil Tolev" notifications@github.com wrote:

Adding this to Wellcome milestone since it does not need to be fixed quickly, but it does need to be fixed before finishing the project.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/CottageLabs/LanternPM/issues/79#issuecomment-237590911, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuXCDSPuovwuVycHb1IvkeCgLshu86rks5qcgZrgaJpZM4JJlCk .

emanuil-tolev commented 8 years ago

Yes, it is, hence why I've put it in. There are two sets of issues:

  1. things which need to be "fine and working ok after fixes are verified" for 2 weeks
  2. this issue, which can be fixed at any time before we invoice, ideally fixed while our contact is still at Wellcome.
markmacgillivray commented 8 years ago

As I now have a general file upload endpoint in the API, I can solve this by checking to see if filereader is available on the current browser:

http://stackoverflow.com/questions/7296426/how-can-i-check-if-the-browser-support-html5-file-upload-formdata-object

If it is not, I will AJAX POST the file itself to a temp dir on the file upload endpoint, with a new uuid as its name. Then I just have to create a new lantern endpoint that can be sent the uuid, and trigger a read of the uploaded file, then conversion to json (API already has code for that too), then delete the file, then send the JSON back down to the UI so that the useful info about the file can be displayed. Then the UI workflow continues as previous.

So because other stuff exists, this would not take too long to complete. But as this was a later issue, I will come back to it. Just adding this note for now to remind myself.

markmacgillivray commented 8 years ago

Actually I decided to try using a shim first to save some hassle with back and forthing to the server. I have used dropfile:

https://github.com/MrSwitch/dropfile

It has been added to the lantern and compliance UIs, and I have tested that nothing has been broken by the addition on the usual browsers. However I cannot check IE.

@emanuil-tolev can you check IE and see if this makes any difference? If you open the devtools or whatever equivalent IE has, if any, you will see the counts in the console of what the js thought it was able to extract from the file, if it worked.

If this approach does not work, I will do the sending the file to the server option instead.

emanuil-tolev commented 8 years ago

Sorry @markmacgillivray , pretty much same situation as it was.

I didn't get any completion emails for the jobs above, just a submission email. I presume that's because there were no records, therefore no processes, so the code that send a completion email couldn't have run. I don't think this is a problem.

emanuil-tolev commented 8 years ago

Oh, if I've not noted it above: Win 7 / IE 8 doesn't work at all, it doesn't seem to be able to execute jQuery 1.10.2, so it doesn't even load the Wellcome UI.

Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; WOW64; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; .NET4.0C; .NET4.0E)
Timestamp: Tue, 9 Aug 2016 11:24:30 UTC

Message: Object doesn't support this property or method
Line: 3
Char: 25571
Code: 0
URI: https://static.cottagelabs.com/d3/d3.v3.min.js

Message: Unexpected call to method or property access.
Line: 5
Char: 25690
Code: 0
URI: https://static.cottagelabs.com/jquery-1.10.2.min.js

EDIT: I don't think we should be fixing this, IE9+ seems reasonable to support. Wellcome Trust seem to use IE9 and above, since they are able to actually load the site, but then get the empty job problem.

markmacgillivray commented 8 years ago

OK thanks. The problem wth the shim approach is that most of them are quite old now too - the market coverage of IE8 is under 5% so most people don't think it's worth supporting. Actually, MS don't support it either. Do we actually know what the oldest browsers wellcome are using are? There may be newer shims for IE10 and up that could work, but even then, we know IE sucks.

Anyway I can do the convoluted option. But can't get back to it until Friday.

On 9 Aug 2016 12:22, "Emanuil Tolev" notifications@github.com wrote:

Sorry @markmacgillivray https://github.com/markmacgillivray , pretty much same situation as it was.

-

Win 7 / IE9:

I filled out the form, but on pressing Submit it just refreshed the page and the inputs were now blank. It went to URL https://compliance. cottagelabs.com/?upload=test_et.csv&email=emanuil@ cottagelabs.com&submit=Submit https://compliance.cottagelabs.com/?upload=test_et.csv&email=emanuil@cottagelabs.com&submit=Submit. I tried this a few times.

On selecting the file with Browse nothing happened on the IE9 DevTools console. However, upon hitting submit and going to this strange URL, there were two messages:

SCRIPT5007: Unable to get value of the property 'match': object is null or undefined jquery-1.10.2.min.js, line 4 character 4961 SCRIPT257: Could not complete the operation due to error 80020101. jquery-1.10.2.min.js, line 4 character 4961

-

Win 7 / IE 10

I filled out the form, and a job was created. It went to 100% immediately. I thought this was because I'd run those identifiers before. However, it turns out the job was empty. https://compliance. cottagelabs.com/#CE8NeA6kzDZFXsJgJ https://compliance.cottagelabs.com/#CE8NeA6kzDZFXsJgJ - just has no results, I assume it simply didn't read in any IDs.

The IE 10 DevTools console reports:

SCRIPT438: Object doesn't support property or method 'readAsBinaryString' compliance.cottagelabs.com, line 123 character 3

-

Win 7 / IE 11

On selecting the file DevTools paused execution on line 123: reader.readAsBinaryString(f); The error displayed under it was Object doesn't support property or method 'readAsBinaryString', as we've come to expect. I submitted the job and got the same as IE 10 - an empty result file. Job: https://compliance.cottagelabs.com/#Xqwf69CFCaNYGvQkv

Win 8.1 / IE 11

Pretty much same as Win 7 / IE 11, though I didn't bother submitting the job at all after I saw the "does not support readAsBinaryString" in the console.

I didn't get any completion emails for the jobs above, just a submission email. I presume that's because there were no records, therefore no processes, so the code that send a completion email couldn't have run. I don't think this is a problem.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CottageLabs/LanternPM/issues/79#issuecomment-238524787, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuXCJYfIGg2pE-6FkC0s12yYQFNl5G_ks5qeGLlgaJpZM4JJlCk .

emanuil-tolev commented 8 years ago

IE8 does not work at all (doesn't load UI), Wellcome must have been using IE9+ since they saw the site. IE9 - IE11 are probably the ones we're targetting with this fix. IE10 and IE11 seem to have the readAsBinaryString support problem, whereas IE9 appears to have an additional problem. I'll ask again about their version of IE, ideally it'll be 10 or 11, not 9.

emanuil-tolev commented 8 years ago

Wellcome have IE11. Getting the readAsBinaryString to work should be enough. It's ok to just have it working just for IE 10 and 11.

  "browser": {

    "name": "IE",

    "version": "11.0",

    "major": "11"

  },

  "engine": {

    "version": "7.0",

    "name": "Trident"

  },

  "os": {

    "name": "Windows",
emanuil-tolev commented 8 years ago

This is the last major issue standing on this project (at the moment...), I've taken care of all other niggling bits and sent an overview to Wellcome. If you've time to check the info I left in #98 and make the email sending code a bit more defensive against network and SMTP errors, that's ideal, but if not, this is the priority one.

markmacgillivray commented 8 years ago

I have made some further changes to the UI to see if we can get it working on IE10 and above, before going for the round-trip option. Can you @emanuil-tolev test again on an IE version?

emanuil-tolev commented 8 years ago

Works in Win 7 / IE 10, Win 7 / IE 11 and Win 8.1 / IE 11 !

emanuil-tolev commented 8 years ago

@richard-jones you can tell Cecy if you like