Change http Timeout error to specify duration
#173
Conversation
drivasperez
force-pushed
the
drivasperez/specify-timeout-ms
branch
from
ecfee48 to
e99ada0
Compare
|
I think this might also need an implementation on the rust host side for the platform. Somewhere like here maybe. When the request times out rust should build the error and add a payload to return to roc. I'm not sure if we need to regenerated glue if we change that tag union, might be able to tweak manually. |
|
Ah, whoops. I'll take a look at finishing this up tomorrow. I was going to ask how the Rust-Roc glue worked anyway, and I suppose this is as good a way to find out as any :) |
Regenerates the glue code for HTTP, moving it into a module and updating the rust code in lib.rs accordingly.
|
Increasingly think the juice is not really worth the squeeze on this one -- I'm a bit worried about breaking something over a relatively minor feature. But it's been a fun learning experience. I'm currently at: have regenerated the The glue generator's format seems to have changed since this was last run, or maybe adding a payload to a single type has pushed it down a different path, so things like module structure and names are quite different. The generated code had a few issues, mostly around not wrapping the new The code is currently breaking on the various SIZE_ASSERT checks, which all seem to be incorrect. I don't know if this is a bug in the glue generator or something I've done wrong, so I'll leave it like this for now. |
|
Thank you @drivasperez Hopefully that is looking ok now. I spent some time working on the glue and made a change to the request body so it is the same as basic-web server now. Fixing up glue manually can be tedious. Need to run now, if there's any issues in CI I can look at them tomorrow. 😃 |
|
🙏
…On Sat, Feb 24, 2024 at 2:11 AM Luke Boswell ***@***.***> wrote:
Merged #173 <#173> into main.
—
Reply to this email directly, view it on GitHub
<#173 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABHX4SUJCULX4M2CCJSYGTYVFD5NAVCNFSM6AAAAABDTEUYL2VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRHEYDSNRXHE3TQMQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
| roc_std = { git = "https://github.com/roc-lang/roc", rev = "7b1f2d2ac1f6e0b7a283cfbe932318f37a0a6861" } | ||
| roc_std = { path = "src/main_glue/roc_std", features = ["std"] } |
There was a problem hiding this comment.
Isn't it risky to copy roc_std to this repo @lukewilliamboswell?
Fixes and changes to it in the main repo will easily be forgotten here.
There was a problem hiding this comment.
I figured it's not so different to locking to a specific revision, and hopefully CI will pickup any significant issues. 🤷♂️ I don't have a strong opinion and am happy to change it back, and also update basic-web server to be the same if you would like.
There was a problem hiding this comment.
Updating is a lot easier with the git rev and it's also nice to have less code in the repo, so I do prefer that approach.
There was a problem hiding this comment.
@Anton-4 I started making the change as you requested to convert back to use roc_std from the roc-lang/roc repository, and it's not a simple change. The issue I see is that the current implementation of glue generates the roc_app crate which depends on roc_std. Two potential options, change glue so that it doesn't generate roc_std and instead links back to the roc repository, or manually update the generated glue files so they also use the roc-lang/roc repo version.
Regarding your comment about having less code, I totally agree and would like to gitignore the generated code but for now there is too much that needs to be manually fixed up. I guess in that vein we could also manually update the roc_app dependency to point to the repository.
I think it is ok to leave it now using the generated glue version, and work towards fixing up glue so that we can eliminate the code altogether from the platforms.
There was a problem hiding this comment.
Sounds good, thank for investigating :)
Closes #147. I guessed that a 64-bit integer is the best type to represent a timeout? I think in Rust you'd use a
Duration, but I don't know if anything similar exists in Roc.