PT download: check "just use a link" with HEAD request#192
PT download: check "just use a link" with HEAD request#192carylwyatt wants to merge 12 commits intomainfrom
Conversation
| return `${request}${newAction}?${params.toString()}` | ||
| }) | ||
|
|
||
| async function checkFetch(url) { |
There was a problem hiding this comment.
@moseshll already reviewed the rest of the code, so I'll just comment on the parts that are new on this branch. Here's an async function with the HEAD request that also sends the error to matomo and updates some variables that will later make sure the user gets an error message and can't click the download button.
| if (isSimpleDownload()) { | ||
| downloadInProgress = true; | ||
|
|
||
| checkFetch(simpleUrl) |
There was a problem hiding this comment.
Adding the checkFetch functionality to the submitDownload function fired by the Download button in the form in the sidebar.
| <div xxstyle="width: 30rem"> | ||
| <div> | ||
| {#if status.percent < 100} | ||
| {#if simpleDownload && downloadInProgress} |
There was a problem hiding this comment.
New error handling in the modal.
| {/if} | ||
| {#if downloadInProgress} | ||
| <span class="btn btn-primary disabled"> Download </span> | ||
| {:else if simpleDownload && downloadError} |
There was a problem hiding this comment.
Adding a "cancel" button if the download has an error.
|
The UI on this needs some additional polishing, but I didn't want to go too far down that path if this isn't a viable option. Thanks! |
|
Thoughts on general implementation:
Trying to think through what we'd learn about failures if we implement the HEAD option:
However:
I'll think about this some more. |
Before I added the After I added the With the
For a valid request, it returned a 200 and all the response headers it would return with a GET request. On this request, I removed the id param, so now I'm getting a 500:
For me, it's more about letting the user know that something went wrong, and the "just a link" failures don't make that obvious since something downloaded (kind of). We get complaints from people that they tried to download and "nothing happened" and we can't recreate that on our end. With this HEAD request step, I can update the UI to tell the user that something went wrong and to try again. This also gives me a place to push the matomo tag with the error code and the URL that was attempted for download so we can track when downloads fail. Without this step, I'm sure there's a way to tell from the logs that someone attempted to reach this URL but was denied... right?
I didn't think of that. But of course that's true! Hmm...
Yes. And in the best case scenario also let the user know the download didn't succeed. |
|
OK -- I think it probably makes sense to go ahead and try the HEAD option, probably combined with some work in imgsrv to optimize HEAD requests. I'm looking at some other things in imgsrv anyway so I can take a look at options there. |
|
For imgsrv I think what we want to do is bail out before generating the actual result if it's a HEAD request -- that is, do all the setup & check access, but don't do the actual generation, for example: https://github.com/hathitrust/babel/blob/main/imgsrv/lib/SRV/Volume/Base.pm#L129 We probably need to put such a check in every |
2 participants
@aelkiss Question for you regarding the new "just use a link" choice for "simple" downloads that don't use the callback function from
imgsrv.If the link is broken/malformed or
imgsrvgives a 403/404/500, there isn't anything we can do with that if it's "just a link." There isn't a way to catch an error or make a suggestion to the user to try again because it's a link, not afetch.We probably don't want to assume that if the link is correct that
imgsrvwill always return what is requested. I think we pursued this path because my implementation wasfetching and returning a blob, and we were concerned about memory/performance if the download/images were large.In my little bit of research on this, I've seen an intermediary option that checks the link via
fetchwith a HEAD request instead of GET request to just see if the request is valid. In the small bit of testing I did on this locally, the HEAD request is slowwww (like 3-4 seconds), but I think that could maybe work since we're using the download modal to "build" the download. In the current implementation, the "build" is instantaneous since it builds the link for the download button on the fly, but if we need a slight delay to send a HEAD request and check if the request is valid before allowing the download (and setting an error in the modal if we get an error), then the download modal is the perfect place to do that.Will an extra request to
imgsrvadd significant problems? Any other gotchas I haven't thought through here?Just some thoughts on this Friday afternoon. I know this process has already taken some time, but this is my last lingering question.
P.S. This is staged on dev-3 so I could test the download.