Skip to content

Conversation

@LMG
Copy link
Collaborator

@LMG LMG commented Oct 24, 2025

The recent upgrade to hyper 1.0 seems to have broken the UI. This commit fixes the issue with hyper_staticfile. It also changes the return types from a fixed Response<Full> to a custom JoshResponse type that allows us to deal with streaming responses instead of always buffering them.
This type and some helper functions are defined in a new hyper_integration.rs file

@christian-schilling christian-schilling added this pull request to the merge queue Oct 27, 2025
github-merge-queue bot pushed a commit that referenced this pull request Oct 27, 2025
* Fix UI

The recent upgrade to hyper 1.0 seems to have broken the UI.
This commit fixes the issue with hyper_staticfile.
It also changes the return types from a fixed Response<Full<Bytes>> to a
custom JoshResponse type that allows us to deal with streaming responses
instead of always buffering them.
This type and some helper functions are defined in a new
hyper_integration.rs file

* Add UI test

Try and download a few things to confirm the UI loads properly

---------

Co-authored-by: Louis-Marie Givel <[email protected]>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 27, 2025
tests/proxy/ui.t Outdated
saving to 'index.html'
index.html 100% |********************************| 633 0:00:00 ETA
'index.html' saved
<!doctype html><html lang="en"><head><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Web site created using create-react-app"/><link rel="manifest" href="/~/ui/manifest.json"/><link rel="icon" type="image/x-icon" href="/~/ui/favicon.ico"><title>React App</title><script defer="defer" src="/~/ui/static/js/main.36e23a1c.js"></script><link href="/~/ui/static/css/main.4a26a351.css" rel="stylesheet"></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html> (no-eol)
Copy link
Collaborator

Choose a reason for hiding this comment

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

tests fail because some resource names (compiled JS) changes. it would perhaps be better to show headers instead of showing full body, and show first e.g. 100 chars/bytes from body with cut or something

tests/proxy/ui.t Outdated
saving to 'favicon.ico'
favicon.ico 100% |********************************| 12014 0:00:00 ETA
'favicon.ico' saved
\x00\x00\x01\x00\x03\x0000\x00\x00\x01\x00\x18\x00\xa8\x1c\x00\x006\x00\x00\x00 \x00\x00\x01\x00\x18\x00\xa8\x0c\x00\x00\xde\x1c\x00\x00\x10\x10\x00\x00\x01\x00\x08\x00h\x05\x00\x00\x86)\x00\x00(\x00\x00\x000\x00\x00\x00`\x00\x00\x00\x01\x00\x18\x00\x00\x00\x00\x00\x00\x1b\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05\x04\x02bV+\x7fq9oe4d^2b]1b]1d^1jc2of2sh2xk5\x82t<\x8d|@\x92\x80A\x95\x84Dzl7\r (no-eol) (esc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

especially here - cut to a few bytes and use hexdump -C

tests/proxy/ui.t Outdated
$ wget http://127.0.0.1:8002/a/repo && cat repo
Connecting to 127.0.0.1:8002 (127.0.0.1:8002)
saving to 'repo'
repo 100% |********************************| 633 0:00:00 ETA
Copy link
Collaborator

Choose a reason for hiding this comment

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

wget progress bar is probably not very useful in those tests; there could be an option to disable it, or just use curl

The recent upgrade to hyper 1.0 seems to have broken the UI.
This commit fixes the issue with hyper_staticfile.
It also changes the return types from a fixed Response<Full<Bytes>> to a
custom JoshResponse type that allows us to deal with streaming responses
instead of always buffering them.
This type and some helper functions are defined in a new
hyper_integration.rs file
Try and download a few things to confirm the UI loads properly
Copy link
Collaborator

@vlad-ivanov-name vlad-ivanov-name left a comment

Choose a reason for hiding this comment

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

\r newlines in cram tests look weird 🤔 other than that lgtm

we should probably swap for e.g. axum at some point to not have to deal with raw hyper, but it's good to have this fixed until then

@LMG LMG added this pull request to the merge queue Oct 29, 2025
@LMG
Copy link
Collaborator Author

LMG commented Oct 29, 2025

I agree about switching to something higher-level, it's something I'd like to investigate in the future.

Merged via the queue into master with commit e063e58 Oct 29, 2025
1 check passed
@LMG LMG deleted the fix_ui branch October 29, 2025 11:29
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

Successfully merging this pull request may close these issues.

5 participants