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

using "client" keyword breaks todo list example #154

Closed
jamescheney opened this issue Mar 28, 2017 · 11 comments · Fixed by #851
Closed

using "client" keyword breaks todo list example #154

jamescheney opened this issue Mar 28, 2017 · 11 comments · Fixed by #851
Labels

Comments

@jamescheney
Copy link
Contributor

If you try to run the todo list example using the appserver (todo.links.txt) then the presence/absence of the "client" keyword on the "todo" function seems to determine whether the program works correctly. If the "client" keyword is used then the page renders as a line or so of gobbledegook, if it is omitted then everything is fine.

I don't know whether this is a bug in the interpreter (i.e. client calls should be allowed in server page handlers and aren't handled correctly) or in the typechecker (i.e. client calls shouldn't be allowed in server page handlers but this isn't being checked). It is also not clear to me whether this is related to the appserver at all, as opposed to realpages or something else.

This is observed with Links 0.6.1. I'm guessing that someone (Sam) ran into and avoided this problem when updating todo.links, so perhaps it duplicates an already-known bug, but here's the issue if not.

@jamescheney
Copy link
Contributor Author

The same thing happens with todo-db.links if client is used with showList, add or remove (all of which do DB calls, but not sure if that is the problem given that this also happens in the non-DB version).

@SimonJF
Copy link
Member

SimonJF commented Jun 16, 2020

@Emanon42 has started to look at this issue. We're not entirely sure what the best solution is, just looking at it.

As has been alluded to in various issues, the problem arises because web pages are now generated on the server prior to being delivered to the client. If, during page generation on the server, a call to a client function is detected, then the continuation is printed as above.

(My guess is that this is a realpages issue rather than a webserver one, since in the original design, the page would consist of a stub, which would call to the server, which could then safely call back to the client).

Fixing this issue is quite tricky; I'm not entirely sure what the best way forward should be. Here are a few ideas:

  1. Raise a runtime error if this occurs. I would hope that it's possible to detect that a client-annotated function is being called from a server context, and this is more informative than what we have at present. That said, it would be nice to do this statically.

  2. Ignore the annotation, and run the function on the server anyway. This would still fail with "true" client functions like the DOM manipulation functions, though, so I guess we would need to add stubs which raise the appropriate runtime errors. I don't like this one as much, since it seems morally wrong to ignore the annotations.

  3. Try and rule this sort of thing out statically. I'm not entirely sure how this would work. One idea could be to extend the type-and-effect system with location information. In short, calls to client-annotated functions would add a client effect, and addRoute would require client to be absent. We'd then allow client calls within HTML, and from within processes spawned using spawnClient.
    While this seems roughly sensible, I'm sure the devil is in the details; does anyone have any thoughts on this design?

I think this has some parallels with some of the issues we've seen with query policy mismatches (c.f. #835), which are currently detected dynamically.

I'd be grateful for any input -- what would people prefer?

@dhil
Copy link
Member

dhil commented Jun 16, 2020

Do option 1 first. It is the least invasive option and I reckon it would yield immediate success. It is also easy to replace later with a more sophisticated solution, if we decide that is what we want.

Option 2 is just silly. Option 3 would require more careful design. Simply adding a client effect is insufficient. For example, you would need ways to abstract over located functions, etc. otherwise you end up with an anti-modular programming model where you would have to define everything twice.

@SimonJF
Copy link
Member

SimonJF commented Jun 16, 2020

Seems sensible. I agree that #3 certainly would need a more careful design.

@jamescheney
Copy link
Contributor Author

It seems like dynamically ruling this out could also rule out lots of things that currently do work. But maybe we should just try it and see.

It would help to have a clear (and minimal) example illustrating the same problem, which we could use to differentially debug comparing what happens with and without the client keyword.

@dhil
Copy link
Member

dhil commented Jun 16, 2020

It seems like dynamically ruling this out could also rule out lots of things that currently do work. But maybe we should just try it and see.

I am not sure what you are hinting at here. Currently we do enforce dynamically that one cannot use a client side function during page generation, it just so happens to yield an extremely unhelpful error.

@SimonJF
Copy link
Member

SimonJF commented Jun 16, 2020

Having looked at it further, I'm fairly sure we can rule this behaviour out at runtime by checking whether is_ajax_call returns true on the current request data from within client_call in evalir. I'd hope this would be precise. @Emanon42 is going to experiment.

@dhil
Copy link
Member

dhil commented Jun 16, 2020

Whatever you decide to implement make sure it is toggleable, i.e. guarded by config/commandline flag.

@SimonJF
Copy link
Member

SimonJF commented Jun 16, 2020

Seems like a routine bug fix rather than a feature to me - is a flag really necessary?

@jamescheney
Copy link
Contributor Author

@dhil sorry, that was a bit oblique. I meant that simply failing if we ever try to call the client from the server is way too strict. But I guess that wasn't what anyone meant.

The "error message" is just an XHR response to a request that doesn't exist; there is no Links code that checks for this error condition. What @SimonJF suggests, checking some server-side state to determine this, seems like the right (dynamic) thing for now.

@dhil
Copy link
Member

dhil commented Jun 16, 2020

Seems like a routine bug fix rather than a feature to me - is a flag really necessary?

I guess you are right if you are going with Option 1 then I agree no flag is necessary. I was thinking about the other options.

Emanon42 added a commit to Emanon42/links that referenced this issue Jun 19, 2020
@Emanon42 Emanon42 mentioned this issue Jun 19, 2020
Emanon42 added a commit to Emanon42/links that referenced this issue Jun 19, 2020
Emanon42 added a commit to Emanon42/links that referenced this issue Jun 19, 2020
@Emanon42 Emanon42 mentioned this issue Jun 19, 2020
jamescheney pushed a commit that referenced this issue Jun 21, 2020
* Fix #154 by checking for ajax request data
jamescheney pushed a commit that referenced this issue Jun 22, 2020
* Fix #154 by checking for ajax request data
frank-emrich pushed a commit to frank-emrich/links that referenced this issue Sep 16, 2020
* Fix links-lang#154 by checking for ajax request data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants