-
Notifications
You must be signed in to change notification settings - Fork 4
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
What should happen if we perform an effect and there is no handle to catch it? #8
Comments
I think it should throw an error, so that every unhandled effect can be caught on |
Consider this. You can't just throw. |
@Jack-Works Are you saying about when If it is preferable to throw an error, I think that is still possible to think about solve situations like that.
Another approach is just return a default value ( |
if you want to do async resume, then you can't do handler check... they're conflict |
It seems that all effects need to be handled, otherwise you get in weird states. It also feels like this is a similar issue to the nested effects. How does it know this block doesn't handle the effect? I think you need to register effect handlers instead, so it knows which one to use. try {
...
} handle (effect === 'foo) {
} handle (effect === 'something') {
} Here is what it desugars to. You could replicate this behavior with a library. const handlers = [];
function handleEffect(handlerCheck, handler, fn, args=[]) {
try {
handlers.push([handlerCheck, handler]);
return fn(args);
} finally {
handlers.pop()
}
}
function perform(effect) {
return new Promise((resolve) => {
for(let i = 0; i < handlers.length; i++) {
const [handlerCheck, handler] = handlers[i];
if (handlerCheck(effect)) {
handler(effect, resolve);
return;
}
}
throw new Error(`No handler for effect ${effect}`);
});
} Here is how to use it async function foo () {
await perform('bar')
}
function handler(effect, resume) {
// Can call function sync or async
resume('Here is the result')
}
// Handled Effect
handleEffect((effect) => effect === 'bar', handler, foo); Nested effects function parentFn() {
handleEffect((effect) => effect === 'bar', (effect, resume) => resume("Result used"), nestedFn);
}
function nestedFn() {
handleEffect((effect) => effect === 'not_foo', (effect, resume) => resume("Not used"), foo);
}
parentFn(); Missing effect foo(); |
Thank you for the ideas, but I don't like so much this syntax: try {
...
} handle (effect === 'foo) {
} handle (effect === 'something') {
} Because it'll now work well with the pattern matching proposal. Also I can't understand very well how it'll avoid to call an effect that isn't handled. For example: function foo () {
const effectName = getUserInput()
perform(effectName)
} So we will still need to think about the situations that launch an effect that isn't handled, because even we register the effects, we'll have these situations. |
The comparison determines if the handle matches. If none of the comparisons
match (all return false), we can throw an error since it is not handled. If
a comparison matches, it is expected to call resume. There is no way to
detect if it never calls resume.
Side Note: The more I mess around with this, the more I feel handle needs
to be an async function instead of a block. That way we can just use the
return value of the function as the result of the effect instead expecting
people to call resume and introducing a new keyword.
…On Wed, Dec 25, 2019 at 1:50 PM Bruno Macabeus ***@***.***> wrote:
I don't like so much this syntax:
try {
...
} handle (effect === 'foo) {} handle (effect === 'something') {}
Because it'll now work well with the pattern matching proposal
<https://github.com/tc39/proposal-pattern-matching>.
Also I can't understand very well how it'll avoid to call an effect that
isn't handled. For example:
function foo () {
const effectName = getUserInput()
perform(effectName)
}
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#8?email_source=notifications&email_token=AGDVLHOQEANVSPQB2CRN563Q2OTQTA5CNFSM4J6TQBU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHUQ7ZI#issuecomment-568922085>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGDVLHJML7IOSQI2CYQDGBLQ2OTQTANCNFSM4J6TQBUQ>
.
|
Oh, okay. Now I understand better your approach. And, yeah, it could be better than what I suggested before, to force to use |
But just a syntax problem: how well we avoid confusion with parameter?
But using this same place to add a condition, we don't have anymore place to add |
it's not weird to not handle an effect. It just likes waiting for the Promise that never resolved. It's the developer's responsibility to check if they have resolved the effect correctly and maybe the JS engine can notify the developer that it is unhandled. If the check is enforced in the language, then we cant do async resume anymore. |
@Jack-Works Yes, it's not unprecedented to not handle an effect, but it's not ideal. I can't think of any cases where that would have been the intended behavior. Otherwise someone would just not call the @macabeus Yes, my proposal somewhat conflicts with the try {
...
} handle (effect if effect === 'some_effect') {
resume 'result';
} It is based on the same syntax Firefox used to have. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch#Conditional_catch-blocks |
@Jack-Works Thank you for the reply, but IMO I'm not agree with that. I prefer to be explicit, otherwise the erros could be hard to debug. try {
...
} handle (true) { // always will catch the effects
} or try {
...
} handle (effect if true) { // if we want to use effect variable inside of this block
console.log(`${effect} launched`)
} @taylor-cedar I liked this approach. It's also a little similar with array comprehensions... but both cases are non-standard and obsolete features. We need to have caution to use it. |
Also what should happen if we not call |
Yep, completely agree. I think this proposal is in the discovery phase, so the exact syntax can evolve. I think the important part is having some kind of matching in the
That is a problem that I would like to solve as well (make it not possible). My feeling would be to force try {
} handle (effect if effect === 'something') {
await doSomethingAsync();
resume 'Result';
} Little weird, but the try {
} handle (effect if effect === 'something') => {
await doSomethingAsync();
return 'Result';
} |
@taylor-cedar There is an issue about use function foo () {
resume 'foo'
}
try {
....
} handle (effect if true) {
foo() // I'm calling resume, but the compiler will can't see it
} Anyway, both cases (enforcing "resume" or using a function) will break if we use a callback: try {
....
} handle (effect if true) => {
setTimeout(
() => 'foo',
1000
) // we want to return 'foo', but it'll return undefined
} |
@macabeus I am suggesting not allowing a callback or called function. Similar to how function wait(time) {
return new Promise((resolve) => {
setTimeout(resolve, time);
});
}
try {
....
} handle (effect if true) {
await wait(1000);
resume 'result';
} If it's a function like, you don't have to worry about resume in other functions, since function foo () {
return 'foo'
}
try {
....
} handle (effect if true) => {
return foo() // No longer using resume. All functions understand `return`
} |
Having a clause condition embedded within the parameter declaration would/should be a separate proposal. That's an entirely different language feature that a) does not currently exist anywhere b) could be applied in multiple places. This type of guards/pattern-matching is available in other languages (eg, Elixir) as a means to avoid Trying to add this here (and only here) does not fit IMO. FWIW, I think unhandled effects should operate like unhandled Promise exceptions: it must throw a runtime error. It's already been suggested here, but with #16 in place, default effect handling is straightforward: // sync
function run() {
try {
foo();
} handle (effect) {
switch (effect) {
case 'foobar':
throw new Error('foobar');
break;
// case '...';
default:
cleanup();
} catch (error) {
// ..
}
}
// async
async function run() {
try {
await foo();
} handle (effect) {
switch (effect) {
case 'foobar':
throw new Error('foobar');
break;
// case '...';
default:
await cleanup();
} catch (error) {
// ..
}
} As shown, handling Also shown is the idea that errors thrown from within the |
There are two more solutions to this. The first one: resume can be async, but it must appear in the sub syntax tree of the handle clause. try...
handle (effect) {
resume await result
} The second one: we can learn from Service Worker. handle(effect) {
effect.waitUntil(result) // the waitUntil or .resume() must be call synchronized
} |
@lukeed I agree that try {
} handle (effect) {
setTimeout(() => {
// Not allowed. resume must be in handle block directly
resume "Something";
}, 10);
} also not allowed function someOtherFn() {
// Not allowed. resume must be in handle block directly
resume "Something"
}
try {
} handle (effect) {
someOtherFn();
} If we also only allow async through try {
} handle (effect) {
// Doing some async
const result = await somePromiseFn();
resume result;
} As for catch after a handle. If we force try {
} handle (effect) {
} catch (error) {
} finally {
} |
If there's to be a I don't like adding |
This whole proposal is about adding a new control flow. How do you go back to the |
The effect's handler finishes execution, which reinforces requiring one. |
How do you pass results of effects back to perform? Also, how would nesting
handles work?
…On Sat, Dec 28, 2019 at 1:56 PM Luke Edwards ***@***.***> wrote:
The effect's handler finishes execution, which reinforces requiring one.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8?email_source=notifications&email_token=AGDVLHOGHAURORF47S7BPI3Q26OOBA5CNFSM4J6TQBU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYQCBI#issuecomment-569442565>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGDVLHOBKDNPTMU4BVSJAFTQ26OOBANCNFSM4J6TQBUQ>
.
|
Can you elaborate "nesting handles"? Is that just when an effect's handler had its own To me, function getName(user) {
return user.name || perform 'ask_name';
}
async function displayName(user) {
const name = await getName(user);
return name.toUpperCase();
}
const arya = { name: null };
try {
const str = await displayName(arya);
console.log(str);
} handle (eff) {
if (eff === 'ask_name') {
return 'Arya Stark'; // sync
// or
return sleep(1e3).then(() => 'Arya Stark'); // async
}
} Runs the same as: function getName(user) {
// return user.name || perform 'ask_name';
return user.name || 'Arya Stark'; // sync
// or
return user.name || sleep(1e3).then(() => 'Arya Stark'); // async
}
async function displayName(user) {
const name = await getName(user);
return name.toUpperCase();
}
const arya = { name: null };
try {
const str = await displayName(arya);
console.log(str);
} handle (eff) {
// not needed
} |
Ok, if I am understanding correctly, you are saying To answer your question quickly without derailing, it's changing who has control over the side effects. Below the caller is deciding what function to call. If you are controlling/writing this code and wanted to change its behavior, you would just change the function that's called, simple.
Let's say this was a library that you didn't control. The library wanted to allow you to implement your own handler (how to get the user name). Generally the pattern that is currently used today is passing in some kind of config that specifies the function you want to use. This proposal is implementing a different mechanism instead. Inside the library:
Outside the library
It also makes testing really nice, you can change your implementation to return a fake value without having to monkeypatch/override your async function. To discuss more, let's create another ticket. |
I was travelling, so I'm just commenting now after read the all answered. Very thank you to help in this discussion! There are many nice suggestions and ideas here, and each idea has different trade offs. I'm doing a recap here, because I think that we are a little mess. Objective: we want to discover the best act when perform an effect and there is no handle to catch it AND avoid problems of Do nothing (first mention here)We always need to think about the simplest solution: do nothing. We could simply return a default value ( function foo () {
const name = perform 'ask_name'
console.log(name)
}
foo() // prints undefined
try {
foo() // prints 'Arya Stark'
} handle (effect) {
resume 'Arya Stark'
} Good:
Bad:
Add clause condition embedded within the parameter declaration (first mention here)try {
...
} handle (effect if effect === 'ask_name') {
...
} So if we perform an effect and no one condition be true, should throw an exception. Good:
Bad:
Doesn't allow to have
|
Thanks for the summary. I think you summed it up nicely. Based on messing around with the syntaxes and limitations you mention above for the last few days, I am strongly in favor of number 3. Here is why:
How will nested // Exceptions
try {
} catch (error) {
if (error === 'something) {
handleError(error);
} else {
// Don't recognize it, rethrow the error so nested catch can get it
throw error;
}
}
// Handles
try {
} handle (effect) {
if (effect.type === 'something') {
const result = async getSomething(effect.id);
resume result;
} else {
// Don't recognize it, let it go to the nested handle
perform effect;
}
} There is a nice symmetry with exceptions. The slight difference is that I would throw an error if resume or perform is not called in |
Third option, without question. Agreed re: resume-vs-return discussion – that's a separate discussion, sorry |
@taylor-cedar I liked the symmetry with exceptions, but with this approach would be hard to compose nested effects, because I always need to explicitly "re-perform" at the end of handle block. It could be bad especially if we are using a third-package and we want to add more effects. I know that my example code could be not so much common, but is just to illustrate the situation: // code on a third package
function thirdPackageFunction (callback) {
try {
callback()
} handle (effect) {
if (effect === 'ask_name') {
resume 'Arya Stark'
}
// since I'm in my package, makes sense to throw an error here
// instead of "re-performing"
throw 'Error! Unknown effect'
}
} function getNameAndAge () {
const name = perform 'ask_name'
const age = perform 'ask_age'
return { name, age }
}
try {
thirdPackageFunction(getNameAndAge)
} handle (effect) {
// FAIL! It will be never called,
// because "thirdPackageFunction" isn't "re-performing"
if (effect === 'ask_age') {
resume 25
}
} MAYBE another approach is implicitly always "re-performing" at the end of handle block is no perform was called, but I don't know how much good is a implicitly behaviour like that...
Yeah, we could discuss about that here: #14 |
@macabeus I agree that you will need to re try {
} catch (error) {
if (error.message === 'somethingKnown') {
// .. do something
} else {
// New exception I don't understand rethrow
throw error;
}
} The way to get around this issue (same for exceptions/handles) is to make the block conditional. try {
} catch (error if error.message === 'somethingKnown') {
// .. do something
} The same fix for both handle and catch work, but that should be a different proposal as I discussed above. |
@taylor-cedar This is a new JS feature & is (should be) completely unrelated to the If you are saying that block-conditionals are necessary in order to this proposal to work, then:
|
We can follow without block-conditionals, but we'll need to think a good approach to compose nested effects. Thinking again now, maybe would be good to always "re-performing" at the end of a |
IMO it should operate the same as Should the effect reach the global scope (meaning it was unhandled), then a runtime error is thrown. |
Sorry guys my previous message above, was meant for @macabeus, not @lukeed. It was a typo. @lukeed I wrote the following.
I am against any matching syntax. I explicitly said that should be a different proposal (not this proposal). I was just showing how it will work in the future after a matching syntax is added, again unrelated, would complement this proposal in the future. @macabeus I agree with @lukeed that bubbling/composing effects should be done by calling |
Okay. It's fine for me. But just one question to ensure if I understand the idea: there is no implicit function sayNameAndAge () {
const name = perform 'ask_name'
const age = perform 'ask_age'
console.log(name, age)
}
function foo () {
try {
sayNameAndAge()
} handle (effect) {
if (effect === 'ask_age') {
resume 25
}
// we explicitly need to re-perform where,
// otherwise "ask_name" will throw an exception
perform effect
}
}
try {
foo()
} handle (effect) {
if (effect === 'ask_name') {
resume 'Arya Stark'
}
} I'll start tomorrow to update the babel plugin with these updates (and write an issue with the changes), so I would like to understand better it. Thank you for the nice ideas =] |
Correct, to behave like The only "drawback" here is that if you forget to manually re- Here's example w/ try-catch and errors. It should be identical if you substitute everything for effects instead of errors: const foo = () => { throw new Error('foo') }
const bar = () => { throw new Error('bar') }
try {
foo();
bar();
} catch (err) {
if (err.message === 'foo') {
console.log('caught: foo')
}
}
// => caught: foo |
Since function sayName () {
const name = perform 'ask_name'
console.log(name) // should print undefined
}
try {
sayName()
} handle (effect) {
// nothing here
} |
Yes IMO. Unless there's no function sayName () {
const name = perform 'ask_name'
// throw new Error('Unhandled effect')
console.log(name) // does not reach here
}
sayName()
//=> Error: Unhandled effect: 'ask_name' |
I'm closing this issue because there is a PR implementing the ideas discussed here. |
@lukeed @taylor-cedar @Jack-Works @otaviopace I know that my code in Babel plugin is very shit (because I'm learning how to write one), but you could take a look on the PR and see the others discussions on TODO section, please? Thank you 😋 |
Sorry, but third option makes these effects whatever but algebraic effects. The whole point is that (unhindered) algebraic effects cover all possible cases of delimited control. There must be an ability to call We already have exceptions, generators, promises and async/await. They are all partial implementations of delimited control, and nobody needs yet another partial implementation in JS. |
What should happen in these situations?
undefined
)?How the others languages solves this problem?
The text was updated successfully, but these errors were encountered: