Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deno fetch fails when destination path can be file also directory #2790

Closed
keroxp opened this issue Aug 17, 2019 · 4 comments
Closed

deno fetch fails when destination path can be file also directory #2790

keroxp opened this issue Aug 17, 2019 · 4 comments

Comments

@keroxp
Copy link
Contributor

keroxp commented Aug 17, 2019

deno fetch https://dev.jspm.io/react-dom
Download https://dev.jspm.io/react-dom
Download https://dev.jspm.io/npm:[email protected]/index.dew.js
Download https://dev.jspm.io/npm:[email protected]/cjs/react-dom.development.dew.js
Download https://dev.jspm.io/npm:react@16?dew
Download https://dev.jspm.io/npm:object-assign@4?dew
Download https://dev.jspm.io/npm:prop-types@15/checkPropTypes?dew
Download https://dev.jspm.io/npm:[email protected]?dew
Download https://dev.jspm.io/npm:[email protected]/tracing?dew
Download https://dev.jspm.io/npm:[email protected]/index.dew.js
Download https://dev.jspm.io/npm:[email protected]/checkPropTypes.dew.js
Download https://dev.jspm.io/npm:[email protected]/lib/ReactPropTypesSecret.dew.js
Download https://dev.jspm.io/npm:[email protected]/tracing.dew.js
Download https://dev.jspm.io/npm:[email protected]/cjs/scheduler-tracing.development.dew.js
thread 'tokio-runtime-worker-6' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 21, kind: Other, message: "Is a directory" }', src/libcore/result.rs:999:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
thread 'main' panicked at 'internal error: entered unreachable code', ../../third_party/rust_crates/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-0.1.22/src/runtime/threadpool/mod.rs:296:26
thread 'tokio-runtime-worker-0' panicked at 'unexpected state while aborting task: Complete', ../../third_party/rust_crates/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/tokio-threadpool-0.1.15/src/task/mod.rs:216:21

I don't know precisely what is going on, but something goes wrong..

Content of ~/Library/Caches/deno/deps/https/dev.jspm.io/npm:[email protected] is:

{"mime_type":"application/javascript"}

but ~/Library/Caches/deno/deps/https/dev.jspm.io/npm:[email protected] is DIRECTORY

This may occur when there are two url modules like these:

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Aug 18, 2019

Panics here:

deno/cli/file_fetcher.rs

Lines 380 to 386 in 4faab6a

dir
.save_source_code_headers(
&module_url,
maybe_content_type.clone(),
None,
)
.unwrap();

Unexpected failing here:

return self.deps_cache.set(&cache_key, json_string.as_bytes());

https://dev.jspm.io/npm:[email protected]?dew does not do any redirect. Instead it just serves a file. Thus in the cache file npm:[email protected] is created, prohibiting any attempt to create a folder of the same name. (thus exploding at npm:[email protected]/tracing)

This feels more like the index.js/index.html scenario to me. Unfortunately jspm chooses direct serving instead of triggering redirect. I am worried that this might become a common problem at some point.

Maybe we should provide a fallback such that when we have a filename vs directory name collision, we move the file (thus making room for the directory) to another name which could still be accessed (e.g. npm:[email protected] file renamed to npm:[email protected]) and try look up this fallback filename every time when we cannot find a file. Whether to do such compromise is honestly debatable, but I am kind of anxious that this problem might be common enough at some point (since there is no required mapping between URL path and file path, and that there isn't really the required concept of directory or file when serving content, comparing to fs)

Another side problem I notice here is that we do not put query params into cache paths (e.g. trailing ?dew is gone when saving local file). Servers obviously can choose to serve different content under different query. @bartlomieju do you think we should encode query as part of the file path here? (I believe '?' is an invalid char in filenames on windows?)

@bartlomieju
Copy link
Member

@kevinkassimo nice catch! There is already an issue for that, ref #2763

@keroxp
Copy link
Contributor Author

keroxp commented Aug 18, 2019

@kevinkassimo Thank you. I think this is quite difficult problem. One simple solution is to store file flatly with full url sha hash (without subdirectories). But this idea seems to have some inconvenience.

@bartlomieju
Copy link
Member

Fixed in #4030, pending v0.34.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants