-
Notifications
You must be signed in to change notification settings - Fork 29
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
Possibility of exception handling in the future? #49
Comments
Also just wanted to throw out the possibility of being able to do something like: using (retry({maxAttempts: 3, backoff: 1000})) {
...
} where let attempts = 0;
while (true) {
try {
...
} catch (e) {
if (++attempts < maxAttempts) {
sleep(backoff);
continue;
}
throw e;
}
break;
} |
I discussed this in another issue, so I'll post the relevant comment here as well:
This isn't something I'm currently considering for this proposal, but may potentially be considered for a follow-on proposal. |
The following was originally posted by @nicolashenry in #74 (comment)
|
This was also discussed in #159, so I will summarize my thoughts from that issue here. There are three different mechanisms we can consider for resource management, though they are not mutually exclusive:
Instead of |
In a way you could consider disposable resources have the following implicit behavior:
The implied [Symbol.enterContext]() {
return this;
} The implied [Symbol.exitContext](hasError, error) {
try {
this[Symbol.dispose]();
} catch (e) {
if (hasError) {
throw new SuppressedError(error, e);
} else {
throw e;
}
}
return false; // let the caller throw 'error'
} |
I'm not that interested in a full-featured context manager like Python, which allows arbitrarily overwriting or swallowing the thrown error of the exiting scope. In some sense the current proposal already allows overwriting error in dispose method, but in a limited fashion. However, I do hope to at least allow |
I don't think passing the error to Also, there are more than a few non-exception related reasons to roll back a database transaction, which is why the example in the explainer assumes rollback unless you mark the transaction as succeeded at the end of the block. |
That approach is very fragile if you bound multiple resources: // roll back transaction if either action fails
async function transfer(account1, account2) {
await using tx = transactionManager.startTransaction(account1, account2);
await using resource = someResource(account1, account2); // resource[Symbol.dispose] throws
await account1.debit(amount);
await account2.credit(amount);
await resource.reduce(amount);
// mark transaction success if we reach this point
tx.succeeded = true;
} // await transaction commit or rollback In this case if the bound |
This seems very uncompelling to me, usually no errors would mean that everything has succeeded fine. For the case of Like it just seems like a hazard that one could change {
const tx = db.transaction("keyval", "readwrite");
const store = db.getStore("keyval");
store.put(3, "counter");
} The spec even specifically says that
A simpler solution would just be to pass a boolean to dispose, I don't see that the error itself would be particularly useful other than just the signal that disposal is because of an error. |
Having written more than my fair share of code leveraging RDBMS and distributed transactions over the years, I've run into plenty of occasions where transaction rollback shouldn't and isn't dependent on exceptions. However, I will say that most of these cases have been in languages where dispose-like semantics have no way to observe whether the transaction was closed due to completion or exception, and in those cases there's usually been an explicit way to signal transaction completion with the default being to rollback. For example, .NET's With respect to IndexedDB, it's transactions aren't aware of exceptions to begin with, and its behavior is to auto-commit, which means the safest approach currently would be to use
I still feel that this introduces more complexity than is justified, and even further weakens the value proposition. I currently favor just recommending using tx = db.transaction();
try {
...
}
catch (e) {
tx.rollback();
throw e;
} It is likely that this what users are already doing today and doesn't introduce confusion as to what to do with an exception when implementing |
In your .NET examples, proper scoping is required. // Create the TransactionScope to execute the commands, guaranteeing
// that both commands can commit or roll back as a single unit of work.
using (TransactionScope scope = new TransactionScope())
{
using (SqlConnection connection1 = new SqlConnection(connectString1))
{
// Opening the connection automatically enlists it in the
// TransactionScope as a lightweight transaction.
connection1.Open();
// Create the SqlCommand object and execute the first command.
SqlCommand command1 = new SqlCommand(commandText1, connection1);
returnValue = command1.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command1: {0}", returnValue);
// If you get here, this means that command1 succeeded. By nesting
// the using block for connection2 inside that of connection1, you
// conserve server and network resources as connection2 is opened
// only when there is a chance that the transaction can commit.
using (SqlConnection connection2 = new SqlConnection(connectString2))
{
// The transaction is escalated to a full distributed
// transaction when connection2 is opened.
connection2.Open();
// Execute the second command in the second database.
returnValue = 0;
SqlCommand command2 = new SqlCommand(commandText2, connection2);
returnValue = command2.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command2: {0}", returnValue);
}
}
// The Complete method commits the transaction. If an exception has been thrown,
// Complete is not called and the transaction is rolled back.
scope.Complete();
} We need to do the same in JavaScript: async function transfer(account1, account2) {
await using tx = transactionManager.startTransaction(account1, account2);
{
await using resource = someResource(account1, account2); // dispose may throw
await account1.debit(amount);
await account2.credit(amount);
await resource.reduce(amount);
}
// mark transaction success if we reach this point
tx.succeeded = true;
} I think |
Another way is to learn from how Golang handles error recovery using interface ErrorConstructor {
recover(): ErrorRecord;
}
interface ErrorRecord {
hasError: boolean;
error: any;
} Calling it would swallow any error thrown in the currently disposing stack. |
The .NET example predates C#'s own addition of // Create the TransactionScope to execute the commands, guaranteeing
// that both commands can commit or roll back as a single unit of work.
using TransactionScope scope = new TransactionScope();
using SqlConnection connection1 = new SqlConnection(connectString1);
// Opening the connection automatically enlists it in the
// TransactionScope as a lightweight transaction.
connection1.Open();
// Create the SqlCommand object and execute the first command.
SqlCommand command1 = new SqlCommand(commandText1, connection1);
returnValue = command1.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command1: {0}", returnValue);
// If you get here, this means that command1 succeeded. By nesting
// the using block for connection2 inside that of connection1, you
// conserve server and network resources as connection2 is opened
// only when there is a chance that the transaction can commit.
using SqlConnection connection2 = new SqlConnection(connectString2);
// The transaction is escalated to a full distributed
// transaction when connection2 is opened.
connection2.Open();
// Execute the second command in the second database.
returnValue = 0;
SqlCommand command2 = new SqlCommand(commandText2, connection2);
returnValue = command2.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command2: {0}", returnValue);
// The Complete method commits the transaction. If an exception has been thrown,
// Complete is not called and the transaction is rolled back.
scope.Complete(); |
Or as follows, if you want to preserve the block scoping: // Create the TransactionScope to execute the commands, guaranteeing
// that both commands can commit or roll back as a single unit of work.
using TransactionScope scope = new TransactionScope();
{
using SqlConnection connection1 = new SqlConnection(connectString1);
// Opening the connection automatically enlists it in the
// TransactionScope as a lightweight transaction.
connection1.Open();
// Create the SqlCommand object and execute the first command.
SqlCommand command1 = new SqlCommand(commandText1, connection1);
returnValue = command1.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command1: {0}", returnValue);
{
// If you get here, this means that command1 succeeded. By nesting
// the using block for connection2 inside that of connection1, you
// conserve server and network resources as connection2 is opened
// only when there is a chance that the transaction can commit.
using SqlConnection connection2 = new SqlConnection(connectString2);
// The transaction is escalated to a full distributed
// transaction when connection2 is opened.
connection2.Open();
// Execute the second command in the second database.
returnValue = 0;
SqlCommand command2 = new SqlCommand(commandText2, connection2);
returnValue = command2.ExecuteNonQuery();
writer.WriteLine("Rows to be affected by command2: {0}", returnValue);
}
}
// The Complete method commits the transaction. If an exception has been thrown,
// Complete is not called and the transaction is rolled back.
scope.Complete(); |
@rixtox I would suggest you read through https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using#using-declaration if you would like to learn more about C#'s adoption of |
What I was getting to is, if I want to roll back if the second In this case what you are claiming is incorrect. Your re-written sample is not "exactly the same". It behaves differently than the original sample when the second public class TransactionScope : IDisposable
{
public void Complete()
{
Console.WriteLine("Complete");
}
public void Dispose()
{
}
}
public class SqlConnection : IDisposable
{
public void Dispose()
{
throw new Exception();
}
}
class OriginalSample
{
static void Main()
{
using (TransactionScope scope = new TransactionScope())
{
using (SqlConnection connection1 = new SqlConnection())
{
}
scope.Complete();
}
}
}
class RewrittenSample
{
static void Main()
{
using TransactionScope scope = new TransactionScope();
using SqlConnection connection1 = new SqlConnection();
scope.Complete();
}
} The original sample won't call The C# compiler knows they have different control flow, and outputs very different compiled code after inlining and dead code elimination: https://godbolt.org/z/1c35365jY If even you can get this wrong, I don't expect any developer can get it right.
Even this document you linked is overlooking this problem and claims that these are equivalent, which they are not. if (...)
{
using FileStream f = new FileStream(@"C:\users\jaredpar\using.md");
// statements
}
// Equivalent to
if (...)
{
using (FileStream f = new FileStream(@"C:\users\jaredpar\using.md"))
{
// statements
}
} It's only true if |
The very fact that the original sample put the using (TransactionScope scope = new TransactionScope())
{
using (SqlConnection connection1 = new SqlConnection())
{
scope.Complete();
}
} If it was in this form, then your rewritten sample would have bahaved the same. But you overlooked the intention behind that original sample, and changed its behavior without noticing it. This is exactly what I want to point out as a danger in adopting RAII. The situation in C# is even worse, because it has two different syntax for {
using (TransactionScope scope = new TransactionScope())
{
using (SqlConnection connection1 = new SqlConnection())
{
connection1.Open();
}
scope.Complete();
}
}
{
using TransactionScope scope = new TransactionScope();
{
using SqlConnection connection1 = new SqlConnection();
{
connection1.Open();
}
}
scope.Complete();
}
{
using (TransactionScope scope = new TransactionScope())
{
using (SqlConnection connection1 = new SqlConnection())
{
connection1.Open();
scope.Complete();
}
}
}
{
using (TransactionScope scope = new TransactionScope())
{
using SqlConnection connection1 = new SqlConnection();
{
connection1.Open();
}
scope.Complete();
}
}
{
using (TransactionScope scope = new TransactionScope())
{
using SqlConnection connection1 = new SqlConnection();
{
connection1.Open();
scope.Complete();
}
}
}
{
using (TransactionScope scope = new TransactionScope())
{
using SqlConnection connection1 = new SqlConnection();
connection1.Open();
scope.Complete();
}
}
{
using TransactionScope scope = new TransactionScope();
{
using SqlConnection connection1 = new SqlConnection();
connection1.Open();
scope.Complete();
}
}
{
using TransactionScope scope = new TransactionScope();
using SqlConnection connection1 = new SqlConnection();
connection1.Open();
scope.Complete();
} |
This is actually a common problem for languages adopting the RAII mechanism. In C++11, destructors are implicitly annotated with In another word, C++ language designers explicitly made exceptions not suppressible. (Not to confuse suppressing error with As a result, many people recommend against throwing from destructors. For example there are many similar arguments in this discussion. There are some defer-like alternatives for C++ like stack_unwinding, folly's |
Even though the proposal currently centers the motivation around resource management, the syntax is also an attractive option for performing other patterns that could also clean up JavaScript code. A couple patterns (that will look familiar to Pythonistas) one might want to use in unit tests are:
The latter would require some means of specifying what to do when an exception is thrown. One way this could look is:
which would behave somewhat like
Note that this isn't a request to modify the current proposal to include this behavior. I'm just raising this now to dissuade an API from being shipped that precludes this functionality, should it be desired in the future.
The text was updated successfully, but these errors were encountered: