arthurfiorette / proposal-safe-assignment-operator

Draft for ECMAScript Error Safe Assignment Operator
https://arthur.run/proposal-safe-assignment-operator/
MIT License
1.07k stars 11 forks source link

This doesn't accomplish anything #35

Open alray2569 opened 2 weeks ago

alray2569 commented 2 weeks ago

I won't beat around the bush: I don't like this proposal. I don't think it achieves any of its stated goals. Let's go through them:

Simplified Error Handling

This is the weakest motivation listed because it provides no explanation of why try-catch blocks are more complicated than "safe assignment," or what exactly safe assignment is supposed to simplify. As far as I'm concerned, this is purely a matter of opinion.

Enhanced Readability

Compare the following code: snippets

try {
    const value = codeThatMightThrow();
    // code that depends on value...
} catch e {
    handleError(e);
}
const [e, value] ?= codeThatMightThrow();
if (e) {
    handleError(e);
} else {
    // code that depends on value...
}

In the first one, all of our application logic is grouped together into the try block, and the error handling is grouped together into the catch block. In the second example, the error handling code interrupts the application logic. By splitting apart logical contexts, we have made the code less readable.

Especially ironic is the claim that safe assignment will reduce nesting. Consider your example code:

async function getData() {
    const response = await fetch("https://api.example.com/data");
    const json = await response.json();
    return validationSchema.parse(json);
}

The most logical error handling in this case is to log the error and return null. And despite much hand-wringing about different types of errors, we don't actually care what error happened because either we got our data or we didn't. So let us fix this code with try-catch:

async function getData() {
    try {
        const response = await fetch("https://api.example.com/data");
        const json = await response.json();
        return validationSchema.parse(json);
    } catch e {
        console.log(`Error getting data: ${e}`);
        return null;
    }
}

Let us now fix the same code with safe assignment (and without the heavyhanded multiple returns in the example solution, since they are both inconsistent with the function contract and introduce flow complexity):

async function getData() {
    let toReturn = null;
    const [fetchError, response] ?= await fetch("https://api.example.com/data");
    if (!fetchError) {
        const [jsonError, json] ?= await response.json();
        if (!jsonError) {
            const [parseError, parsed] ?= validationSchema.parse(json);
            if (!parseError) {
                toReturn = parsed;
            } else {
                console.log(`Error getting data: ${parseError}`);
            }
        } else {
            console.log(`Error getting data: ${jsonError}`);
        }
    } else {
        console.log(`Error getting data: ${fetchError}`);
    }
    return toReturn;
}

This could hypothetically be mitigated by some sort of safe-coalescing operator, say, ?&. (For people who are interested the gritty theoretical details, this would turn the safe assignment pattern into a monad with unit = value => [null, value] and bind = (a, b) => a ?& b.) In practice, this would look like this:

async function getData() {
    const [error, parsed] ?= await fetch("https://api.example.com/data")
        ?& async response => await response.json()
        ?& json => validationSchema.parse(json);
    if (error) {
        console.log(`Error getting data: ${jsonError}`);
        return null;
    } else {
        return parsed;
    }
}

If we also added a safe-fallback operator, say, ?|, we could specify a default value inline, for example:

async function getData() {
    return await fetch("https://api.example.com/data")
        ?& async response => await response.json()
        ?& json => validationSchema.parse(json)
        ?| error => (console.log(`Error getting data: ${jsonError}`), null);
}

Now we're actually getting into the realm of being more readable than try-catch. (By the way, if this code looks strangely familiar, that's because it's basically the same thing as .then/.catch for promises, but with a uniform operator instead of a bespoke set of methods.

BUT — ?& and ?| pretty much obviate ?=. We can use the former pair to chain and handle errors before we even make an assignment. So this is really starting to look like an alternative proposal now. You'll notice that ?= is gone from my example.

Consistency Across APIs

Ah, yes, a classic: Let us achieve consistency across APIs by giving API developers yet another way to be inconsistent. Need I say more?

Improved Security

This only improves security if we break every existing API to enforce the use of safe assignment. As long as I can forget to use safe assignment, it's not safe. And of course, if we rewrite things like async/await and parsing to enforce safe assignment, we break decades of code. Certainly, this could be enforced by a linter, but so could try-catch.

Do you know what is enforceable? Just returning an error-value tuple outright. And that doesn't require any new magical syntax, either. Of course, all other arguments above still stand for returning a tuple.

khaosdoctor commented 2 weeks ago

Valid arguments, a good write up, but I disagree on a few things.

The nested code will probably not be written like this, just look at how they do it in Go... I'd do it like this:

async function getData() {
    const [fetchError, response] ?= await fetch("https://api.example.com/data");
    if (fetchError) return console.log(`Error getting data: ${fetchError}`);

    const [jsonError, json] ?= await response.json();
    if (jsonError) return console.log(`Error getting data: ${jsonError}`);

    const [parseError, parsed] ?= validationSchema.parse(json);
    if (parseError) return console.log(`Error getting data: ${parseError}`);

    return parsed
}

else is (most of the time) an useless construct of any language.

Now this:

async function getData() {
    return await fetch("https://api.example.com/data")
        ?& async response => await response.json()
        ?& json => validationSchema.parse(json)
        ?| error => (console.log(`Error getting data: ${jsonError}`), null);
}

Is elegant, I'd love to have it as pattern matching in JS, but it is unreadable.

Consistency Across APIs

JavaScript is not, was not, and will probably never be consistent. I talked about this on #24:

My point is, there's no point on trying to unify and make the library consistent now, 29 years later, when we can't deprecate any old function. If that was the case, parseInt would've been out for ages and jQuery would've been forgotten.

Do you know what is enforceable? Just returning an error-value tuple outright. And that doesn't require any new magical syntax, either. Of course, all other arguments above still stand for returning a tuple.

But that would require a MASSIVE change to all the previous APIs that you just said cannot be broken. I'd love that JS had Go-like (or even Python's) multiple returns with enforced error handling, but it can't, it simply is not possible not because of the technical side, but because of the operational side.

alray2569 commented 2 weeks ago

Ah, see, not I'm not a fan of conditional early returns because I think they make functions harder to understand quickly, and in that regard, else is very often useful. I'm also not a fan of explicitly returning the output of a void function call, so I would object to return console.log(...). Finally, I note even still in your multiple-return example, you've written essentially the same line of code three times.

The hypothetical ?& and ?| operators are a generalized version of .then and .catch, so you can think of that block as something like this:

async function getData() {
    return await fetch("https://api.example.com/data")
        .then(async response => await response.json())
        .then(json => validationSchema.parse(json))
        .catch(error => (console.log(`Error getting data: ${jsonError}`), null));
}

Obviously, that code doesn't actually work, but that's the idea. You could also think of it like the | and || operators in a shell. In a pseudo-JavaScript-shell, you might think of it like this

fetch "https://api.example/data" \
    | response => toJSON response \
    | json => parseSchema validationSchema json \
    || echo "Error getting data: $?"

This also behaves similarly to Haskell's Either. In Haskell:

fetch "https://api.example.com/data" 
    >>= (\response -> toJSON response) 
    >>= (\json -> parseSchema validationSchema json)
    `fromRight` print "Error getting data"
khaosdoctor commented 2 weeks ago

I understand, but in your first example, the code is way harder to read with all the nested conditionals than repeating the same line three times, in those cases I think we should leave the DRY a bit to the side because it gets way more complex than just repeating the line.

But in the end that’s a matter of opinion and depending on the type of language you’re using it is the only way. Go does it like this for example.

On Tue, 27 Aug 2024 at 00:43 Andrew Ray @.***> wrote:

Ah, see, not I'm not a fan of conditional early returns because I think they make functions harder to understand quickly, and in that regard, else is very often useful. I'm also not a fan of explicitly returning the output of a void function call, so I would object to return console.log(...). Finally, I note even still in your multiple-return example, you've written essentially the same line of code three times.

The hypothetical ?& and ?| operators are a generalized version of .then and .catch, so you can think of that block as something like this:

async function getData() { return await fetch("https://api.example.com/data") .then(async response => await response.json()) .then(json => validationSchema.parse(json)) .catch(error => (console.log(Error getting data: ${jsonError}), null));}

Obviously, that code doesn't actually work, but that's the idea. You could also think of it like the | and || operators in a shell. In a pseudo-JavaScript-shell, you might think of it like this

fetch "https://api.example/data" \ | response => toJSON response \ | json => parseSchema validationSchema json \ || null

This also behaves similarly to Haskell's Either. In Haskell:

fetch "https://api.example.com/data"

= (\response -> toJSON response) = (\json -> parseSchema validationSchema json) fromRight default

— Reply to this email directly, view it on GitHub https://github.com/arthurfiorette/proposal-safe-assignment-operator/issues/35#issuecomment-2311234057, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYNMMAX5QCYKD4TQM7OKVDZTOVPTAVCNFSM6AAAAABNEBLFK6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJRGIZTIMBVG4 . You are receiving this because you commented.Message ID: <arthurfiorette/proposal-safe-assignment-operator/issues/35/2311234057@ github.com>

alray2569 commented 2 weeks ago

Ah, yeah, I don't like that example with the nested conditionals, either. That was the point. You're going to have a hard time convincing me that the most readable version of all of the examples isn't the one that uses try-catch.

khaosdoctor commented 2 weeks ago

But that’s the point. If you don’t like it you don’t have to use it. This won’t be replacing try statements, you can continue to write them as you would. It’s just another option to make the code more succinct like promises and callbacks.

DScheglov commented 2 weeks ago

@khaosdoctor

But that’s the point. If you don’t like it you don’t have to use it.

Unfortunately we are working not in the vacuum, the option to use ?= will require an additional effort, either:

So, unfortunatelly, it is not just my choice. It could be a team choice, but the temptation is too high.

figloalds commented 2 weeks ago

I'd say that both things have value in the programming space depending on how the author intends it But a large try block with multiple points of failure is not as useful as you seem to believe it is The intentionality behind a code like this:

async function getData() {
    const [fetchError, response] ?= await fetch("https://api.example.com/data");
    if (fetchError) return console.log(`Error getting data: ${fetchError}`);

    const [jsonError, json] ?= await response.json();
    if (jsonError) return console.log(`Error getting data: ${jsonError}`);

    const [parseError, parsed] ?= validationSchema.parse(json);
    if (parseError) return console.log(`Error getting data: ${parseError}`);

    return parsed
}

Is extremely powerful and significantly easier to debug than a ginormous function with one single catch expression.

My concern about this feature is this, suppose we have a code like this

const [err, result] = await fetch(`/api/people/${selectedPerson.id}`);

But selectedPerson is somehow undefined?

Will this syntax wrap the entire right-side expression in error handling or will it behave weird and throw? And what is the actual expectation for this scenario?

alray2569 commented 2 weeks ago

@figloalds I assume you meant:

const [err, result] ?= await fetch(`/api/people/${selectedPerson.id}`);

The proposal states that the ?= operator "is compatible with promises, async functions, and any value that implements the Symbol.result method." This would suggest that your example would continue to throw an error as the error happens before a value is returned to the ?= operator.

Of course, anyone who is paying attention will also recognize that this is error, because something like await fetch("https://arthur.place") doesn't actually produce any of those types, but rather a Response object. Following the normal resolution of operators, the await operator would resolve before ?= and throw an UnhandledPromiseRejection before ?= has a chance to deal with it.

So the text of the proposal is inconsistent with its code examples. Looking at the polyfill raises yet further concerns. The preprocessing is inconsistent, and in fact, the following lines are preprocessed to the same code:

const [error, value] ?= something;
const [error, value] ?= something();

// both yield
const [error, value] = something[Symbol.result]();

This is obviously concerning, but let us set that aside for the moment. How does it treat await? Well, it's also inconsistent, since the ?= preprocessor skips over the await to inspect the function or Promise directly before resolving the await operation like so:

const [error, value] ?= await something;
const [error, value] ?= await something();

// both yield
const [error, value] = await something[Symbol.result]();

This implies that the treatment of the await operator is specially magical and that your example will indeed still throw unless some additional magic is added.

DScheglov commented 2 weeks ago

@alray2569

If I understand correctly, the syntax couldn't be polifilled at all.

Regarding the ?= operator, I consider it syntactic sugar for:

<left-hand-expr> ?= <right-hand-expr>:

let __temp_result_tuple__N;

try {
  __temp_result_tuple__N = [undefined, <right-hand-expr>]
} catch (error) {
  __temp_result_tuple__N = [
    error ?? new ReferenceError(`Nullish value has been thrown: ${error}`),
    undefined
  ]
}

<left-hand-expr> = __temp_result__N;

But the proposal involves misusing the Symbol.result, which confuses how it works with any kind of composition:

const div = (a, b) => {
  if (b === 0) throw new Error('Div by zero');
  return a / b;
}

const [err, value] ?= div(1, 0) + div(1, 0);

I cannot understand if it is equal to:

const [err, value] = div[Symbol.result](1, 0) + div[Symbol.result](1, 0);

... I'm really confused with this polifill.

Perhaps, @arthurfiorette can clarify ...

alray2569 commented 2 weeks ago

@DScheglov That's pretty much exactly my hangup, too. If it was just syntactic sugar for try-catch and had the same semantics as try-catch, that would make it much less confusing, but that doesn't seem to be the proposal as it's currently written. As written, there are three special cases:

All three of these cases are handled slightly differently by the proposal, which would create a parsing nightmare (#38), and all uses that don't match one of these three cases throw a TypeError... which is an interesting choice for something that's supposed to reduce the need for try-catch.

When you put those things together, you get seemingly absurd results (#39), most poignant in the following example, where the first of these lines is fine, but the second throws a TypeError even though the RH values by themselves have identical parse trees and behavior:

const [e1, v1] ?= await (Promise.resolve(4)); // ok
const [e2, v2] ?= await Promise.resolve(4);   // TypeError

Plus, these three lines would all behave differently:

const [e1, v1] ?= await fun();   // => await fun[Symbol.result]()
const [e2, v2] ?= await (fun()); // => await fun()[Symbol.result]()
const [e3, v3] ?= (await fun()); // => (await fun())[Symbol.result]()
JamesRamm commented 1 week ago

The hypothetical ?& and ?| operators are a generalized version of .then and .catch, so you can think of that block as something like this:

async function getData() {
    return await fetch("https://api.example.com/data")
        .then(async response => await response.json())
        .then(json => validationSchema.parse(json))
        .catch(error => (console.log(`Error getting data: ${jsonError}`), null));
}

Obviously, that code doesn't actually work, but that's the idea. You could also think of it like the | and || operators in a shell. In a pseudo-JavaScript-shell, you might think of it like this

Actually, this code pretty much works. You don't need to wrap it all in async/await

function getData() {
     return fetch("https://api.example.com/data")
         .then(response => response.json())
         .then(json => validationSchema.parse(json))
         .catch(error => console.log(`Error getting data: ${jsonError}`));
}

Don't forget that async/await is just syntactic sugar on top of the original Promise API, which is superior IMO anyway and has further useful stuff like all, allSettled, withResolvers...

My opinion was that the Promise API was a really good start on a nice fluent api and instead of improving it further, async/await was tacked on to the language, muddying the waters and resulting in confused proposals like this one....

DScheglov commented 1 week ago

@JamesRamm ,

Don't forget that async/await is just syntactic sugar on top of the original Promise API, which is superior IMO anyway and has further useful stuff like all, allSettled, withResolvers...

actually the async/await was a syntactic sugar only with babel ... then it became a language feature, and the code written with .then has an absolutely different stack, then code written with async/await.

My opinion was that the Promise API was a really good start on a nice fluent api and instead of improving it further, async/await was tacked on to the language, muddying the waters and resulting in confused proposals like this on

I don't see any relation between the async/await and this proposal. More then, in some meaning this proposal is counter to the async/await. The last one is about moving declarative .then-based code to the imperative code, keeping the monadic nature of Promises (yes, Promise doesn't follow Haskel's contracts, but it is still possible to define Monad with Promise), this approach takes imperative version and tries to transform it to the declarative one, moving the error to the main control flow and breaking the Monadic nature of exceptions (yes, it is also possible to define the Monad with exceptions in terms of Category Theory)

khaosdoctor commented 1 week ago

@DScheglov That's pretty much exactly my hangup, too. If it was just syntactic sugar for try-catch and had the same semantics as try-catch, that would make it much less confusing, but that doesn't seem to be the proposal as it's currently written. As written, there are three special cases:

  • RH is an awaited Promise
  • RH is an awaited async function call
  • RH is an object with a [Symbol.result] method

All three of these cases are handled slightly differently by the proposal, which would create a parsing nightmare (#38), and all uses that don't match one of these three cases throw a TypeError... which is an interesting choice for something that's supposed to reduce the need for try-catch.

When you put those things together, you get seemingly absurd results (#39), most poignant in the following example, where the first of these lines is fine, but the second throws a TypeError even though the RH values by themselves have identical parse trees and behavior:

const [e1, v1] ?= await (Promise.resolve(4)); // ok
const [e2, v2] ?= await Promise.resolve(4);   // TypeError

Plus, these three lines would all behave differently:

const [e1, v1] ?= await fun();   // => await fun[Symbol.result]()
const [e2, v2] ?= await (fun()); // => await fun()[Symbol.result]()
const [e3, v3] ?= (await fun()); // => (await fun())[Symbol.result]()

I think the written proposal is outdated tbh, people have been chatting about it on #5 #4 and others, apparently the reached conclusion was to remove ?= over try <exp>

ljharb commented 1 week ago

@DScheglov async/await is indeed almost entirely syntactic sugar, and is not a complete replacement for Promises. The stack is irrelevant since that’s not part of the language (yet).

khaosdoctor commented 1 week ago

@JamesRamm ,

Don't forget that async/await is just syntactic sugar on top of the original Promise API, which is superior IMO anyway and has further useful stuff like all, allSettled, withResolvers...

actually the async/await was a syntactic sugar only with babel ... then it became a language feature, and the code written with .then has an absolutely different stack, then code written with async/await.

My opinion was that the Promise API was a really good start on a nice fluent api and instead of improving it further, async/await was tacked on to the language, muddying the waters and resulting in confused proposals like this on

I don't see any relation between the async/await and this proposal. More then, in some meaning this proposal is counter to the async/await. The last one is about moving declarative .then-based code to the imperative code, keeping the monadic nature of Promises (yes, Promise doesn't follow Haskel's contracts, but it is still possible to define Monad with Promise), this approach takes imperative version and tries to transform it to the declarative one, moving the error to the main control flow and breaking the Monadic nature of exceptions (yes, it is also possible to define the Monad with exceptions in terms of Category Theory)

I'll have to agree in parts, I do agree that the proposals follow different ideas of changing the flow order, but in essence, the goal is also to simplify the written part of the language (callback hells, promise hells etc)

DScheglov commented 1 week ago

@DScheglov async/await is indeed almost entirely syntactic sugar, and is not a complete replacement for Promises. The stack is irrelevant since that’s not part of the language (yet).

The first of all noone points that async/await is a replacement for promises. It is obvious that async/await utilise promises.

The second, why is a stack irrelevant?? And why is it important to be a part of language to be relevant??? From the practical point of view it is relevant, especially when we are talking about error handling.

The third, let's talk about what the syntactic sugar is. If it is processed by compiler (including the jit one) in the same way. So, if it is a syntactic sugar or not depends on the compiler or interpeter. As instance for the V8, the async/await is not a syntactic sugar for the promises: https://v8.dev/blog/fast-async

And finally the language feature from the Language point of view could be considered as a syntactic sugar if and only if the language specification directly requires the language feature to be implemented exactly as a syntactic sugar

DScheglov commented 1 week ago

@khaosdoctor

I think the written proposal is outdated tbh, people have been chatting about it on #5 #4 and others, apparently the reached conclusion was to remove ?= over try <exp>

Just replace in the examples ?= with try <expr> and you will get the same concerns -- the behaviour of the "safe assignment" operator is not concistent.