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

fix: allow Raw as UnixfsType #327

Merged
merged 1 commit into from
Oct 12, 2022
Merged

fix: allow Raw as UnixfsType #327

merged 1 commit into from
Oct 12, 2022

Conversation

ramfox
Copy link
Contributor

@ramfox ramfox commented Oct 11, 2022

Acknowledges that a raw node is a valid UnixfsNode and therefore can be resolved as a file in the gateway

closes n0-computer/beetle#148

Acknowledges that a `raw` node is a valid `UnixfsNode` and therefore can be resolved as a file in the gateway
@ramfox ramfox added the fix Fixes a bug label Oct 11, 2022
@ramfox ramfox added this to the v0.1.1 milestone Oct 11, 2022
@ramfox ramfox self-assigned this Oct 11, 2022
@ramfox ramfox requested review from Arqu and dignifiedquire October 11, 2022 22:20
@ramfox ramfox modified the milestones: v0.1.1, v0.1.0 Oct 11, 2022
Copy link
Collaborator

@Arqu Arqu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@Arqu Arqu merged commit 275d88b into main Oct 12, 2022
@Arqu Arqu deleted the ramfox/resolver_raw_fix branch October 12, 2022 10:26
@dignifiedquire
Copy link
Contributor

@ramfox I don't think this is quite correct. IPLD Raw is something else than Unixfs Raw. We shouldn't treat them the same.

Asfaiu the current kubo gateway allows unixfs paths -> other ipld blocks which is likely why it works on their side. If we start allowing for that we should explicitly resolve IPLD, not treat IPLD as unixfs

@ramfox
Copy link
Contributor Author

ramfox commented Oct 12, 2022

Okay great, thank you. I had a feeling this was off so I wrote up an alternative way of doing this here. But I'm also woefully ignorant of the way the gateway should ideally operate.

Before this PR, the path to get to the bug was this:
The request comes in.
The get_handler determines that the ResponseFormat is ResponseFormat::Fs (most likely because this seems to be a kind of default).
The get_handler matches on the response format & calls serve_fs, which errors when the unixfs_type of the resolved node is found to be None (hence the "couldn't determine unixfs type" error).

So the problem is either:

  1. we are incorrectly assuming that the response format should be ResponseFormat::Fs, and instead should be ResponseFormat::Raw. Fixing the ResponseFormat type, would call serve_raw rather than serve_fs. Problem solved? (@Arqu ???)
  2. serve_fs should actually just be able to handle OutType::Raw (the above-linked solution). We handle OutType::Raw & the problem is solved.
  3. There is some alternative way we should be handling raw blocks that we don't have set up yet that I need clarity on

@Arqu
Copy link
Collaborator

Arqu commented Oct 12, 2022

1. we are incorrectly assuming that the response format should be `ResponseFormat::Fs`, and instead should be `ResponseFormat::Raw`. Fixing the `ResponseFormat` type, would call `serve_raw` rather than `serve_fs`. Problem solved? (@Arqu ???)

Technically the ResponseFormat is correct and line by line is pretty much also what kubo does. Where we seem to differ is that we don't handle the calls well so we potentially want to diverge and redirect that.

Not sure whether we can correctly "detect" this state, but given we can return OutType::Raw as the body we can just make a special case next to dir and file handling.

@ramfox
Copy link
Contributor Author

ramfox commented Oct 12, 2022

@Arqu something like this?: #332

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fixes a bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

bug: "couldn't determine unixfs type" for large portion of successfully resolved requests
3 participants