-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
alternative rewrite of the motivating example with existing language features #9
Comments
agreed |
My general problem with the pattern in its most general form is it allows skipping an error check while not providing a compelling syntax improvement over regular error checking. Consider: const [error, result] ?= example()
if (error) {
// Handle error
} else {
// Use result
} vs: try {
result = example()
} catch(error) {
// Handle error
} There is essentially no difference for this regular case. The one with the proposed const [, result] ?= example()
// use result This leaves the code in an undefined state. Even if you don't use try/catch the code would throw and let you know the underlying error instead of "eating" it. It is true that typescript could save the day when you don't check for error, but not everyone have to use it. This should be independent of any external type system. Let me also try (no pun intended) to go over the important objections for regular try catch: // Nests 1 level for each error handling block
async function readData(filename) {
try {
const fileContent = await fs.readFile(filename, "utf8")
try {
const json = JSON.parse(fileContent)
return json.data
} catch (error) {
handleJsonError(error)
return
}
} catch (error) {
handleFileError(error)
return
}
} This is not structured correctly, see this equivalent code (error types are made up here): async function readData(filename) {
try {
const fileContent = await fs.readFile(filename, "utf8")
const json = JSON.parse(fileContent)
return json.data
} catch (error) {
if (error instanceof JSONError) {
handleJsonError(error)
} else if (error instanceof FileError){
handleFileError(error)
}
}
} with the proposed syntax: async function readData(filename) {
const [readError, fileContent] ?= await fs.readFile(filename, "utf8")
if (readError) {
handleFileError(error)
return
}
const [jsonError, json] ?= JSON.parse(fileContent)
if (jsonError) {
handleJsonError(error)
return
}
return json.data
} It is almost identical with the added complexity of sprinkling error checks throught the code flow. Arguably that might be better to keep the locality of the information but that's something you have to do even if you just want to merge all of them to a single error handler. So I agree with the @seansfkelley here, existing language features are already good enough to handle very similar cases. I don't see any compelling reason why Edit: I have misread the handler functions incorrectly at first Edit: the try/catch version also has the advantage of automatically bubbling the error up (also see the incomplete approach at #16) |
I really want to be proven wrong here. This comes up so often that I started to believe I am missing something very obvious. |
It seems like the core improvement provided is that of removing a block scope and avoiding let result;
try {
result = doStuff();
} catch (e) {
handleError(e);
return;
} can instead avoid non-constants and some amount of scopes/nesting: const [e, result] ?= doStuff();
if (e) {
handleError(e);
return;
} Or put another way: the The reference to Rust in the proposal is telling; Rust's Result is a monad(-ish? I'm not an expert) with syntax-level support, but JavaScript already has a monad-ish with syntax-level support, which is Promises. True that you can't use them for synchronous work, but I don't think the juice is worth the squeeze. Perhaps what's really missing from JavaScript is blocks-as-values. ;)
|
I think it becomes harder to combine errors for the
If you factor in the possibility of skipping the error check by mistake -thus silencing it- it is not worth it just to skip a few curly braces. Declare then initialize is avoidable if you lift the try/catch higher. try {
const result = doStuff();
// use result here, no need to leave the scope to consume some intermediate value.
} catch (e) {
// handle all errors here, or get rid of try/catch to delegate to the caller
} Generally it is just better to have a single try/catch higher in the tree. Using it multiple times for each and every throwable is kind of an anti-pattern to me. OTOH, indeed there are some cases where you just don't care about the error and you'll need a lone try/catch, but there are already solutions to those. For example: const resultWithDefault = await doStuff().catch(() => "default value"); or if it is sync, with a simple helper: const resultWithDefault = maybe(() => doStuff(), "default value"); Edit: fixed sync example to defer calling the function |
I want to emphasize that writing less code doesn't necessarily mean writing better code. While brevity can be useful, it's important to consider that the JavaScript interpreter may end up doing more work, analyzing more code, or facing new challenges. Introducing a new operator like ?= could lead to more documentation, a steeper learning curve, and additional complexity for developers. Furthermore, many languages use try-catch, and that knowledge is reusable, reinforcing the idea that familiar patterns should be favored. For more on this perspective, see #24. |
Only if the functions being called explicitly tells you what classes they throw. For example, you have no way to distinguish where a |
this helper is impossible to exists, because |
You're right, that's my bad. Should have not call the function there. For the unknown error type case can you provide a practical example where this new feature would be useful? |
Just a general comment: the automatic bubbling of thrown exceptions is "nice" in one sense, but the actual experience of it is that every level either needs a try..catch, or the exception sort of hijacks the flow control. IMO the main value of this proposal is allowing code to communicate "errors" that bubble up a call-stack, but without that being an automatically flow-control-interrupting signal (or requiring the extra overhead of try..catch at each step). In the FP world, when we deal with a type like Either (basically what this [ err, data ] tuple is), we have the ability to explicitly decide when to fork flow control to handle an error, versus just performing other tasks and returning the Either value on up the stack. |
I see the potential value, but I don't think this is framed as such. My main issue with this is that -contrary to the original intention- people will see this as a way to skip error handling. If there was a way to enforce handling of the error or bubbling it up at a language level, I wouldn't mind how it "looked" much. Exposing the error in a tuple just makes it a regular value that you can throw away. The new language feature should provide more guarantees if we want to create a monadic error handling mechanism, rather than just handing us an array. |
Absolutely not, this feature is a opt-in way to get your errors as values to treat them. If you want to ignore the error you can already do that using Also and obviously, alongside with ignoring the error, you will NOT have the function result, so people won't be able to ignore errors easier, but instead are now able to handle it easier and in places they wouldn't put a try/catch before. Just count of how many times you saw someone using |
Catching the error with a catch block is very explicit in that you're using an error handling mechanism. Here, we are hiding that fact behind a "safe assignment". Getting this accepted as an error handling construct is very difficult. Destructure the array and you have a value. The fact that it's representing an error is very abstract unless everyone accepts that as a common pattern akin to return values representing errors in C. Most of the time if I want a catch block with a small scope, I probably want a "default value". JSON.parse is probably the best example. Still, a
How so? Maybe this is the part I'm missing. If I don't check the error, there is nothing stopping me from trying to access the value. For an occasional error I may not even realize this until it happens. |
That indeed might not be automatically enforced, but... that's not supposed to be how you construct and consume such values. In other words, you don't return both a success and an error, you return one or the other. The position in the tuple here just helps you identify the value designation (error vs success). Again to reference Either from the FP world, the value is either a Left (error) or a Right (success); it's impossible to be both at once. How I might use something like that would be: // was there an error?
if (result[0] != null) {
// do some alternate or cleanup work
// now, pass this result back up the call-stack
// for further processing
return result;
}
else {
// handle result[1] success value
} The contention I'm making is, this sort of thing, where you're checking whether the result was success or error, and performing some different operations as a result, WITHOUT the normal processing having been interrupted by an exception, and requiring a |
How can you access the value? Please show me an example |
You're right that the helper mentioned doesn't work due to doStuff() being evaluated before maybe(), but the approach I suggested in issue #24 does work effectively. Your proposal, as an error-handling strategy or design pattern, is solid and not entirely new—it's a good practice. However, the real issue is that the language already allows us to achieve the same results without adding anything new. Complicating the language might be unnecessary when we can implement this pattern with existing features. |
List of accepted proposals that could be achieved with previous language features: |
This is from the proposal: // This suppresses the error (ignores it and doesn't re-throw it)
const [, data] ?= fn() Another way of doing it: const result ?= fn()
doSomethingWith(result[1])
That makes it a convention rather than a language feature IMHO. This is somewhat a contrived example but consider this: // A well behaving function, considers result to be an Either:
const inner = () => {
const result ?= underlyingFunction();
if (result[0] != null) {
// Propagate state
return result;
}
else {
// handle success
}
}
// In a different place, someone uses this function:
const result = inner();
// I have no idea this was an Either anymore:
useValue(result[1]); The consumer of
Even though this is indeed very attractive -and I agree there is some value- I still disagree that such a language feature should only convert your return value/error into an array. Especially when it is straightforward enough to make similar helpers. I think this #22 (comment) explains it much better than I could. I am not sure how it can be achieved but this new operator is far away from what Rust's |
You are overthinking. const inner = () => {
// propagate error
const result = underlyingFunction();
// handle success
} |
IMO, consuming code should get to decide whether and how to respond to an error, not the code that may produce an error. That's why "exceptions" are considered by most to be side effects, because the "side effect" they have is interrupting the flow control of the caller, just to communicate the error up to it. It's not a small fact that being able to handle errors and values with the same level of flow-control makes code more predictable, than when errors are elevated to this shouting "Interrupt everything!" type of behavior. Your objection to the convention over enforcement is actually another way of saying the same thing: you want the producer to control/enforce that the consumer must have known about the error. It seems to bother you that the calling code is able to cover up or dismiss errors. But that's precisely what a separation of concerns (in levels of abstraction) promises... that the concerns of the consumer and the concerns of the producer are separate, and shouldn't be tightly coupled. |
And with TS having the correct typing for return type of
We can discriminate doesn't mean we MUST. Especially with JS. So, that is your example: TS Playground The |
minor off-topic: Many Many thanks for your Functional Light JS Book!!!
Using the |
Additionally regarding the
The initial intent of objection is to say that @anacierdem , please correct me if I incorrectly got your intention. |
This, I agree. I’m not opposing this idea but I also believe it is possible to bubble the error up without interrupting the control flow of the caller. I just want to make sure the developer is 100% aware about they are handling an error. I think there are two important requirements here:
Unfortunately the current proposal doesn’t fulfill any. The first is not possible because the proposal is about the place we do the assignment and doesn't involve the caller context. The second is also problematic because the "error handling" is being delegated to a basic if statement or skipped array element, which is not something anyone would relate to error checking by default. These facts also limits or risks the possible future extensions around incrementally adding these additional safeguards. Let's say we wanted to implement enforcement in the future, how would we define it? Would we say "there should be an if statement checking if the error value is truthy", what about if there are additional conditions?
I don't think adding more constraints around how an error can be manipulated is coupling the producer of values/errors to the consumer. It will instead strenghten the decoupling as you can build your producer knowing the language will make sure an error value you produce is always handled gracefully.
This is a js feature, we shouldn't depend on/assume anything else IMHO. Again there's someone else that can put things into words better than me: #32 I get that this is not aiming at solving the things I described and they are probably the topic of another proposal. But still, this specific proposal is not even building towards them, and that's my main objection. If we can at least have a glimpse of such a thing built atop, I will be fully onboard. The bottomline is, for such a weak (not fulfilling my requirements and arguably hard to extend) assignment operator, I don't think we need a language level feature considering achieving the exact same thing is almost trivial (even without exceptions). If there must absolutely be a syntax for this, it is too opinionated as apparent from other discussions like #30. I think I am going towards "agree to disagree" at this point. I definitely see the value here but the tradeoffs are not worth it to make this a feature in my view. Edit: P.S @getify functional light js is one of my favourites as well. Learned a lot from it ❤️ |
"Readable" is subjective and should also factor in considerations like syntactic consistency. The objection raised by myself and others is that this structure is inconsistent with existing language constructs and would likely lead to a unnecessary divergence in code style and attendant developer overhead/tooling complexity for a trivial savings in keystrokes. Or, to quote someone from upthread:
Moving along...
What do you mean by this? If you're referring to the type of the // declare as non-nullable type
let value: T;
try {
value = doStuff();
} catch (e) {
// handle, but realistically you probably don't need to let `e` escape this block
}
// compiler will make sure `value` is actually non-null before you try to use it |
I mean that your example definitely does not the same as // declare as non-nullable type
let result: [{}, undefined] | [undefined, T];
try {
result = [undefined, doStuff()]
} catch (e) {
result = [
e ?? new Error("Nulish value is thrown from doStuff"), // yes it must ensure that e is not null or undefined
undefined
]
}
const [error, value] = result; The initial your example with chained promise definitelly does a different thing. |
My example doesn't do the same thing exactly because, with thrown errors, one usually does not need to continue the main body of the function after they are caught. That's why my example says
if I wanted to shoehorn Result-style error handling into try-catch at the call site, then yes, your translation would be more accurate. But the point I was trying to make is that I don't:
And if I did, I would use the pattern mentioned upthread, here simplified to match the case we're talking about: const [error, value] = maybe(() => doStuff(thing1, thing2)); While I don't generally like thunks, I don't think their attempted removal here justifies new syntax and new language semantics for everybody when a utility method like the above is so easy to write: function maybe(thunk) {
try {
return [undefined, thunk()];
} catch (e) {
return [e, undefined];
}
} |
to use the caught If we don't need I like your
I'm absolutely agree that to write |
Example: https://github.com/arthurfiorette/tinylibs/blob/476fea076cbac0e43b531be5255a06fe6ceb1f8b/packages/fast-defer/src/create-deferred.ts#L5 |
Actually it is not a relevant example. |
As written, the motivating example
could instead be rewritten to something along the following lines
without introducing new syntax.
It's slightly awkward with the
?.
and== null
checks, but it's plenty clear that it's doing a series of individually-failable operations with their own error handling (then
alternating withcatch
), and treats null/undefined as the early-abort sentinel for downstream operations.I don't have an alternate motivating example to provide, but I did want to raise that this one doesn't strike me as all that motivating.
The text was updated successfully, but these errors were encountered: