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's the code pattern for passing through non-cleanup-related thrown errors? #137
Comments
I'd use a wrapper: using stack = new DisposableStack;
const db = stack.adopt(dbGetter(), () => { try { db[Symbol.dispose](); } catch {} });
return db.doAThing(); |
Thanks @bakkot that's some neat code. What I was trying to get at with this issue, though is it just seems like the tail wagging the dog for the errors thrown by the intentionally hidden automated cleanup code to suppress the, probably much more important, error in the explicitly visible code. This is following the precedent set by In this case, though, the cleanup code is likely built into the objects you're using and totally ignorant of the context where they are being used. It seems really unexpected and undesirable that errors thrown from them should override an error or return value from the "real" work. Unfortunately, I don't have an alternative to suggest, but, am I the only one who sees this as undesirable? |
I'd think the general expectation is that cleanup code simply should not throw exceptions. When it does, you'll have a real problem to handle, which is just as important as a "suppressed" error from the guarded part. Either way, to ignore such errors I'd write function ignoreDisposalErrors(res) {
const orig = res[Symbol.dispose];
res[Symbol.dispose] = function() {
try {
orig.apply(this, arguments);
} catch {}
};
return res;
}
// This method should throw a DBError, which ultimately comes from the database object, if it fails
function doAThing(dbGetter) {
using db = ignoreDisposalErrors(dbGetter());
return db.doAThing();
} |
@bergus is correct. Exceptions thrown from dispose generally should indicate a serious problem (i.e., failure to close a file handle, failure to commit or rollback a database transaction, etc.) and are often just as important as the exception from the body, if not more so. |
I would also add to @bakkot's example with something just as reusable as @bergus's example, but without mutating the resource: function tryDispose(obj) {
try {
obj?.[Symbol.dispose]();
} catch {}
}
using stack = new DisposableStack();
const db = stack.adopt(dbGetter(), tryDispose);
return db.doAThing(); |
Suppose I have a method that does something that may result in a thrown error of a specific type.
I'm writing a wrapper around it that should just pass that thrown error through unchanged.
My wrapper also needs to get and cleanup a resource.
Now, suppose the resource is disposable and I want to make use of that.
As near as I can tell this is what I would have to write.
That is maybe a bit better than having to do a
try
nested in afinally
,but it still seems pretty awkward.
Is there a better way to write this with the current version of the proposal?
Is this a reasonable use-case?
The text was updated successfully, but these errors were encountered: