What's the code pattern for passing through non-cleanup-related thrown errors?

[brad4d]

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.

// This method should throw a DBError, which ultimately comes from the database object, if it fails
function doAThing(dbGetter) {
  const db = dbGetter();
  try {
    // Pass through either the result or any thrown error
    return db.doAThing();
  } finally {
    // any error thrown in the finally overrides an error thrown in the try, so I have to catch them.
    try {
      db.cleanup();
    } catch (cleanupErr) {
      // I don't care about cleanup errors
    }
}

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.

// This method should throw a DBError, which ultimately comes from the database object, if it fails
function doAThing(dbGetter) {
  try {
    using db = dbGetter();
    return db.doAThing();
  } catch (err) {
    // dig the actual database error out, ignoring any cleanup errors.
    while (err instanceof SuppressedError) {
      err = err.suppressed
    }
    throw err;
  }
}

That is maybe a bit better than having to do a try nested in a finally, 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?

[bakkot]

I'd use a wrapper:

using stack = new DisposableStack;
const db = stack.adopt(dbGetter(), () => { try { db[Symbol.dispose](); } catch {} });
return db.doAThing();

[brad4d]

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 try / catch /finally, but in that case the catch and finally blocks are at least explicitly visible parts of the local context and it makes a sort of sense that they should be able to override an error thrown from the try block.

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?

[bergus]

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();
}

[rbuckton]

@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.

[rbuckton]

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();