MystenLabs / walrus-sites

Walrus Sites: Decentralized Websites using Sui and Walrus.
https://docs.walrus.site/walrus-sites/intro.html
Apache License 2.0
9 stars 6 forks source link

Better handling of empty files (Error: the data is empty) #88

Open kkomelin opened 2 weeks ago

kkomelin commented 2 weeks ago

When I try to publish my frontend app to Walrus Sites, I receive the following error:

2024-06-19T07:21:52.356360Z  INFO site_builder: initializing site builder
2024-06-19T07:21:52.356711Z  INFO site_builder: configuration loaded config=Config { portal: "walrus.site", package: 0x514cf7ce2df33b9e2ca69e75bc9645ef38aca67b6f2852992a34e35e9f907f58, general: GeneralArgs { rpc_url: None, wallet: None, walrus_binary: Some("walrus"), walrus_config: None, gas_budget: Some(500000000) } }
2024-06-19T07:21:52.356759Z  INFO site_builder::util: Using wallet configuration from /home/kos/.sui/sui_config/client.yaml
Error: running the command exited with error: 2024-06-19T07:21:59.801087Z  INFO walrus: running in JSON mode
2024-06-19T07:21:59.801167Z  INFO walrus_service::cli_utils: Using Walrus configuration from /home/kos/.walrus/client_config.yaml
2024-06-19T07:21:59.801233Z  INFO walrus_service::cli_utils: Using wallet configuration from /home/kos/.sui/sui_config/client.yaml
2024-06-19T07:21:59.801637Z  INFO walrus_service::cli_utils: Using RPC URL set in wallet configuration
Error: the data is empty
kkomelin commented 2 weeks ago

Reproduction demo is in progress. Will publish here shortly

mlegner commented 2 weeks ago

I suspect you have an empty file somewhere. But I agree, we should improve the handling of that and/or print a more useful error.

Related to #65.

kkomelin commented 2 weeks ago

The reproduction:

Clone the reproduction repo:

git clone -b walrus-deploy https://github.com/kkomelin/sui-dapp-starter
cd sui-dapp-starter

Install the deps and run the deploy:

pnpm i
pnpm --filter frontend deploy:walrus
kkomelin commented 2 weeks ago

If it matters, I've got robots.txt empty in the root. But it's intentional.

mlegner commented 2 weeks ago

If it matters, I've got robots.txt empty in the root. But it's intentional.

That makes total sense. Walrus currently doesn't support empty files, so we either need to add support for that or handle it on the Walrus Sites side.

Until we have that, can you just add a comment line to that file as a workaround?

kkomelin commented 2 weeks ago

Yes, good idea. Will try now

kkomelin commented 2 weeks ago

Hooray! It works! https://suidappstarter.walrus.site/

Just in case you need to have some more info: Object ID: 0xe5e34c4fc294b5ac95984b1f9bd834a9365386df7478460ea6459526b4161596 Url: https://5q9p8r84khdpdia3dnsw0pkpk3rxa0ic2kqbdlstne8vu2t192.walrus.site/

Thank you very much for you help!

giac-mysten commented 2 weeks ago

Thanks @kkomelin , this is a known "area of improvement" :)

That makes total sense. Walrus currently doesn't support empty files, so we either need to add support for that or handle it on the Walrus Sites side.

I would say for now the best fix would be to catch empty files in the site-builder and print a message saying which are the empty files, and either to add something to them or remove them. The same could be done for unknown extensions.

kkomelin commented 2 weeks ago

Thanks @giac-mysten, As for empty files, I agree with your solution. As for .webmanifest extension, it's just a JSON. Browsers just use the extension to set higher level mime type for it. So I would still insist on adding support for it by default if it's easy to do.

giac-mysten commented 2 weeks ago

So I would still insist on adding support for it by default if it's easy to do.

of course! We just need to have a clear boundary of files/file types we support directly + a good fallback strategy, so that we don't end adding a new file type every day :)

But this will be surely fixed.

kkomelin commented 2 weeks ago

@giac-mysten Yeh, I understand. And it's important to collect more requests for this particular file extension from the community before implementing it.