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

POC wasm-friendly library version (not ready to merge) #60

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dylanowen
Copy link
Contributor

I've been wanting the ability to embed dot-http in the browser so that I can use it from an extension I'm working on. I was also thinking about #46 but in the context of using something like mdBook to place/run sample dot-http requests inline in rendered markdown documentation.

This POC splits apart dot-http into a library (that works with wasm) and the cli portion and it seems to work pretty well!

I've included an example under examples/wasm to play around with.

Open Questions

This is just a POC, but with some feedback I think it could be merged into the main codeline. These are my main open items / questions.

I Changed Request to http::Request

This isn't strictly required and I made the change a in a different commit so that it could be merged separately, if it makes sense. My rationale was to make it easier to include dot-http as a library to interact with other libraries such as yew: https://github.com/yewstack/yew/blob/master/yew/src/services/fetch/web_sys.rs#L268 and Reqwest: https://github.com/seanmonstar/reqwest/blob/master/src/async_impl/request.rs#L507.

This has some breaking changes to the output of existing dot-http scripts because of how http::Request serializes:

  • headers don't match the case from the server, they are lower-cased
  • the status code capitalization has changed Ok to OK

Publishing / Naming

Should dot-http-lib be dot-http and the cli be dot-http-cli? Either way I'm not sure how to deal with dual publishing a library and a cargo-delivered application from the same repo. Any advice for naming and how to make this work nicely would be appreciated.

Library API

I did the bare minimum or library organization, but I feel like if we actually published a library form of dot-http it'd be good to cleanup and better organized the exposed API.

Errors

I know we recently moved to anyhow which is great for the application errors, but it'd be great to bring back the error enums for the library portion (maybe using thiserror?). It would give the wasm portion more insight into the errors (I'm imagining using CodeMirror and displaying the parse errors inline)

JS Questions

client.global

Does client.global.get do anything? Aren't we usually resolving using the variables declared in the global scope with this[<key>] = <value>?

Should tokens in requests be expressions?

I noticed that you can write something like

some-header: {{var test = "example"; test}}

If this is a use case, it definitely would be powerful to keep, but I think we could simplify the JS Scoping problems (more info below) if it wasn't.

Sandboxing

I don't imagine a wasm implementation would need full sandboxing, but it still is nice to have an environment we can cleanup / restart. One way we could do that would be something like an "Evalbox" but since we'd be sending messages to the iframe we'd need the functions on the ScriptEngine to be async. This could probably be mitigated with changing the scoping (below) so we wouldn't need to bring async in.

Scoping

With the current setup it's very hard (maybe impossible?) to keep the scope of the script engine outside of the global scope. This is fine for js engines that we can control completely, but with the browser we just have to dump everything into the global scope, which could have some strange interactions with the page / previous requests. For example saving a variable named client causes some issues as the declare function overwrites the client from init.js.

Example JS sent to a Script Engine, each code-block is a new evaluation:

var _env = {};
var _snapshot = {};
var client = {
  global: {}
};
client.global.set = function (key, value) {
  _snapshot[key] = value;
};
client.global.get = function (key) {
   if (_snapshot[key] != undefined) {
     return `[key];
   }
   if (_env[key] != undefined) {
     return _env[key];
   }
   return null;
};

^ this is our first issue, we're referencing _env and _snapshot which (if we weren't in the global scope) live in a different scope. We can work around this though as it's all code we own.

var response = {"body":"{\n  \"field\": \"value\",\n  \"next\": \"example\"\n}","headers":{"accept-ranges":"bytes","content-length":"43","content-type":"application/json; charset=UTF-8","date":"Mon, 14 Dec 2020 05:47:10 GMT","etag":"W/\"2b-c6YnSpm3P1324kV4Wl7tDfjGVE8\"","x-powered-by":"Express"},"status":200};
response.body = {"field":"value","next":"example"};
client.global.set('next', response.body.next);

Starting our next request

this['next'] = "example";

^ this is our main issue, setting next on this only makes it accessible to our token resolvers if they assume they're operating on the gobal scope as any variable resolution not on the global scope ignore this. Something like

const context = {};
function evaluate(script) {
    eval(script)
}
evaluate.call(context, `this["next"] = "example";`)
evaluate.call(context, `console.log(next)`)

wouldn't work because we resolve next from the current scope, where it doesn't exist and then check the global scope skipping our this object. but

const context = {};
function evaluate(script) {
    //  https://www.ecma-international.org/ecma-262/5.1/#sec-10.4.2
    eval.call(window, script)
}
evaluate.call(context, `this["next"] = "example";`)
evaluate.call(context, `console.log(next)`)

would because our this is the window / global scope.

Possible Solution

If we were to change how the tokens are resolved (to not be expressions, but rather just references). We could change our token resolution call from a full execute_script call to execute_script(&format!("client.global.get(\"{}\")", token)). This way we could customize our context away from the global scope while still allowing for the convenient form of {{token}}.

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.

1 participant