q2view says a tutorial file is too large

fedarko · September 26, 2019, 5:47am

Hi! I was going through the importing tutorial and noticed that the emp-paired-end-sequences.qza "output artifact" breaks q2view if you click on the "view" button next to it—

This issue can be reproduced by just following this URL to q2view that points to the problematic artifact: QIIME 2 View

This doesn't seem like that big of a file (pretty sure I've thrown heavier stuff at q2view in the past :), so I'd be curious as to why this error is happening. But it's not a large issue at all.

edit: looks like this problem impacts a few of the other artifacts listed on the page, including emp-single-end-sequences.qza, and demux-single-end.qza. (Interestingly, demux-paired-end.qza seems ok?)

thermokarst · September 26, 2019, 2:00pm

Hey there @fedarko!

I just loaded the link up on FF68:

I wonder if for some reason the Accept-Ranges header wasn't being set appropriately when you checked earlier (not sure off the top of my head if that is strictly server-supplied, or if the client has to somehow negotiate that header):

github.com

qiime2/q2view/blob/d149b6dc18d3b45db927e22298d09ed5f99ac435/app/init/resolve.js#L28-L35

      
        
            if (headers.has('Accept-Ranges') && headers.get('Accept-Ranges') === 'bytes') {
                // TODO: be smarter with the Ranges header
                return fetch(rawSrc.data).then(res => res.blob());
            } else if (contentLength === null || contentLength <= 15 * 1024 * 1024) {
                return fetch(rawSrc.data).then(res => res.blob());
            }
            throw Error(`File is too large (${contentLength} bytes) and the` +
                        ' server does accept the "Ranges" header');

When I load that link right now the Accept-Ranges header is set (which means the size check branch is skipped).

fedarko · September 26, 2019, 8:31pm

Hi @thermokarst! Oh man, I think this might be another funky browser issue.

The link works for me in Firefox (version 68.0.1) also—but I consistently get this error in Chrome, both in version 77.0.3865.90 (on a macOS laptop) and in version 77.0.3865.92 (on an Android phone).

I just checked and the error also occurs in Safari (version 12.1.2). The problem persists in incognito/private mode in Chrome/Safari, also. That this error persists across multiple devices and browsers leads me to think that this isn’t an intermittent issue.

In the Chrome browser console, I get pretty much the same error as is displayed on the page: Uncaught (in promise) Error: File is too large (32997106 bytes) and the server does accept the "Ranges" header at resolve.js:34.

Similarly, the Safari JS console gives me the error Unhandled Promise Rejection: Error: File is too large (32997106 bytes) and the server does accept the "Ranges" header. (The listed line here, though, is index.js:126.)

I’m not really sure what to do with this information… my guess is the non-Firefox browsers are maybe handling the Accept-Ranges header in a idiosyncratic way (or vice versa :)?

Thanks for your help with this!

thermokarst · October 1, 2019, 4:55pm

thermokarst · October 2, 2019, 11:29pm

thermokarst · October 2, 2019, 11:29pm

ebolyen · October 12, 2019, 12:40am

Sorry I lost track of this topic.

Worth mentioning, the ranges header is kind of a red-herring here, in that even if the server did support it, nothing useful would happen as the zip library we use doesn’t have a way to abstract over the IO so I can’t really proxy seek calls against a remote server via ranges.

That said, browsers should be agreeing on which branch to take in this code so something should probably change to make the requests similar in different browser’s networking stacks (I hope a plain old XMLHTTPRequest will just “fix” this).

We can also probably increase the cap from 15MiB which is kind of low nowadays.

ebolyen · November 19, 2019, 6:54pm

I’m sorry if this original message came off as rude, it wasn’t my intention, but re-reading it can definitely be seen as a little dismissive, so again I apologize.

Looking at the code again, I don’t see a very compelling reason to have any of this branching (especially if browsers are interpreting it differently like @fedarko found) . I think originally the goal was to prevent the browser from giving an OOM exception as the JS heap is only around 4gb, but it seems like that isn’t worth worrying about at this point.

We should still figure out how to do actual range-requests someday, which would let us handle arbitrarily large visualizations (as long as those visualizations can stream data). But until then, I made a PR stripping the above logic:

ebolyen · November 19, 2019, 6:54pm

fedarko · November 19, 2019, 8:30pm

@ebolyen I don’t think your original message sounded rude! I really appreciate you all taking the time to handle the weird issues like this that QIIME 2 users like me run into

The main reason I reported this issue to you all was that it seemed like a general problem that’d impact people following the tutorials – so alleviating that problem (for now, at least) sounds great.