如何修复“本地捕获异常的抛出”错误？

Question

在这个处理REST API调用的函数中，任何一个被调用的函数来处理请求的部分可能会抛出错误以表示应该发送错误代码作为响应。但是，函数本身也可能发现错误，在这种情况下它应该跳转到异常处理块中。

static async handleRequest(req) {
    try {
        let isAllowed = await checkIfIsAllowed(req);
        if (!isAllowed) {
            throw new ForbiddenException("You're not allowed to do that.");
        }
        let result = await doSomething(req); // can also raise exceptions
        sendResult(result);
    } catch(err) {
        sendErrorCode(err);
    }
}

Webstorm将使用以下消息下划线标记throw：本地捕获异常。此检查报告任何JavaScript throw语句的实例，其异常始终被包含在try语句中捕获。使用throw语句作为“goto”来更改局部控制流可能会令人困惑。

但是，我不确定如何重构代码以改善情况。

我可以从catch块中复制粘贴代码到if检查中，但我认为这会使我的代码更难阅读和维护。

我可以编写一个新函数，进行isAllowed检查，并在不成功时引发异常，但这似乎是绕过问题，而不是解决Webstorm据说报告的设计问题。

我们是否以不良方式使用异常，这就是为什么我们遇到了这个问题，还是Webstorm错误只是误导性的，并且应该禁用？

- cib

---

@matchish 嘿 - 刚刚注意到这个赏金。我不确定你认为我的答案的哪个部分违反了DRY原则？我唯一能想到的就是 sendErrorCode - 但它没有逐字重复; 在一个地方，它从这段代码块发送一个非常具体的错误，在更通用的 catch 中，它发送一个还没有编码的更一般化的错误...？ - James Thorpe

Answer 1

与詹姆斯·索普的观点相反，我略微更喜欢抛出异常的模式。我没有看到任何强制性的理由来将 try 块中的本地错误与来自调用堆栈深处的错误区别对待...只需将它们抛出即可。在我看来，这是更好的一致性应用。

因为这种模式更加一致，所以在您想要将 try 块中的逻辑提取到另一个函数中时（该函数可能位于另一个模块/文件中），它自然更容易进行重构。

// main.js
try {
  if (!data) throw Error('missing data')
} catch (error) {
  handleError(error)
}

// Refactor...

// validate.js
function checkData(data) {
  if (!data) throw Error('missing data')
}

// main.js
try {
  checkData(data)
} catch (error) {
  handleError(error)
}

如果您在try块中处理错误而不是抛出它，则如果您将其重构到try块之外，则逻辑必须更改。

此外，处理错误的缺点是需要记住尽早返回，以便在遇到错误后不继续执行try块中的逻辑。这很容易被忘记。

try {
  if (!data) {
    handleError(error)
    return // if you forget this, you might execute code you didn't mean to. this isn't a problem with throw.
  }
  // more logic down here
} catch (error) {
  handleError(error)
}

如果你关心哪种方法更高效，那就不应该。处理错误在技术上更高效，但两者之间的差异绝对微不足道。

考虑到WebStorm在这里可能过于以自己的观点为中心。ESLint甚至没有这个规则，所以任何一种模式都完全有效。

- J. Munson

---

我同意，但我认为这取决于具体情况。在所有可能的错误被相同处理的情况下，我认为这样可以使代码更易读。 - Albert James Teddy

这应该是答案。 - CommonSenseCode

同意。这只是那些毫无意义的荒谬标准之一。 - m4heshd

如果您有自己的自定义错误类，有时候将不同类型的错误抛出并在一个位置捕获它们，然后根据它们的类型进行不同的处理会更好。 - rooby

好的回答已经在这里了，没有必要为了取悦编辑而过度努力。 - SamAko

Answer 2

如果isAllowed失败，您正在检查某个内容并抛出异常，但是您知道在这种情况下该怎么做-调用sendErrorCode。如果您不知道如何处理情况，例如在异常情况下，请向外部调用者抛出异常。

在这种情况下，如果发生这种情况，您已经定义了相应的处理过程-直接使用它，而不是间接的抛出/捕获：

static async handleRequest(req) {
    try {
        let isAllowed = await checkIfIsAllowed(req);
        if (!isAllowed) {
            sendErrorCode("You're not allowed to do that.");
            return;
        }
        let result = await doSomething(req); // can also raise exceptions
        sendResult(result);
    } catch(err) {
        sendErrorCode(err);
    }
}

我可以把catch块中的代码复制粘贴到if检查中，但我认为这会使我的代码不易于阅读和维护。

相反，正如上面所述，我希望处理这种情况的方式是这样的。

- James Thorpe

---

如果你不知道如何处理特殊情况，就应该向外部调用者抛出异常。如果这是真的，那么就解释了问题所在。然而，这意味着我们在整个项目中都错误地使用了异常处理，而不仅仅是在这个函数中。由于没有其他答案，我会接受这个答案。 - cib

James，我同意这个答案，但是一个朋友给我发了这个链接：https://dev.to/nedsoft/central-error-handling-in-express-3aej 你觉得呢？ - fsljfke

如果你只需要执行"sendErrorCode()"并返回，那么这很好。但是，如果你需要在处理或清理错误时进行多个操作，那么就会出现重复代码。如果你的try块中有多个位置可以检测到内部错误，则情况会变得更糟。如果你必须在catch块之后执行其他工作，无论是否存在错误，如果你在try块中返回，就没有办法到达这里。 - wojtow

@wojtow，你的场景听起来可以通过重构来处理... - James Thorpe

Answer 3

由于这不是一个阻塞错误，而仅仅是一个IDE的建议，因此这个问题应该从两方面考虑。

第一方面是性能。如果这是一个瓶颈，并且有可能在编译或转移到新版本的nodejs时使用它，那么重复的存在并不总是一个不好的解决方案。看起来IDE在这种情况下提供了提示，这样的设计在某些情况下可能导致优化不佳。

第二方面是代码设计。如果它会使代码更易读，简化其他开发人员的工作 - 那就保留它。从这个角度来看，上面已经提出了一些解决方案。

- SlowSuperman

Answer 4

在try块中，返回一个拒绝的promise，而不是抛出错误。

  try {
    const isAllowed = await checkIfIsAllowed(request);

    if (!isAllowed) {
      return Promise.reject(Error("You're not allowed to do that."));
    }

    const result = await doSomething(request);

    sendResult(result);
  } catch (error) {
    throw error;
  }

- Ayush Shrestha

---

请说明您的解决方案相较于其他方案更优越的理由。 - iron9

@iron9 这只是一个更干净的替代方案，与此处提到的解决方案相比。 - Ayush Shrestha

将一个被拒绝的 Promise 返回是否比抛出错误更好的做法？ - CodeConnoisseur

@CodeConnoisseur 使用其中一个与使用另一个没有优势。您可以在此处阅读更多信息：https://dev59.com/LlwX5IYBdhLWcg3wvRkf#33446005 - Ayush Shrestha

Answer 5

这里有一个关于“为什么不将异常用作正常流程控制”的好回答，在这里。

不应该抛出本地可捕获的异常的原因是您在本地知道如何处理该情况，因此，根据定义，它并不是异常情况。

@James Thorpe的回答对我来说很好，但@matchish认为它违反了DRY（Don't Repeat Yourself）原则。我认为一般而言，它并没有违反DRY原则。DRY原则是由创造该短语的人定义的，“系统中每个知识片段必须具有单一、明确、权威的表达形式”。在编写软件代码方面，它意味着不要重复复杂的代码。

实际上，任何被认为违反DRY原则的代码都可以通过将重复的代码提取到一个函数中，然后从以前重复的位置调用该函数来“修复”。让代码的多个部分调用sendErrorCode是解决DRY问题的解决方案。所有关于如何处理错误的知识都在一个明确的地方，即sendErrorCode函数。

我会稍微修改@James Thorpe的回答，但这更像是一个小问题而不是真正的批评，即sendErrorCode应该接收异常对象或字符串，但不能同时存在：

static async handleRequest(req) {
    try {
        let isAllowed = await checkIfIsAllowed(req);
        if (!isAllowed) {
            sendErrorCode(new ForbiddenException("You're not allowed to do that."));
            return;
        }
        let result = await doSomething(req); // can also raise exceptions
        sendResult(result);
    } catch(err) {
        sendErrorCode(err);
    }
}

更重要的问题是错误发生的可能性有多大，以及将 !isAllowed 视为异常是否合适。异常用于处理不寻常或不可预测的情况。我认为 !isAllowed 应该是一种正常情况，应该使用特定于该情况的逻辑进行处理，而不像突然无法查询数据库以获取 isAllowed 问题的答案那样。

@matchish 的提议的解决方案更改了 doSomethingOnAllowedRequest 的契约，从永远不会抛出异常变成了经常抛出异常，将异常处理的负担放在所有调用者身上。这很可能会导致 DRY 的违反，因为它会导致多个调用者具有相同的错误处理代码重复，所以从抽象的角度来看，我不喜欢它。实际上，这取决于整体情况，例如有多少调用者，他们是否实际上共享对错误的相同响应。

- Old Pro

---

如果您需要记录错误，您应该修改两行代码，这就是为什么我认为它不符合DRY原则的原因。 - Serhii Shliakhov

你写的 !isAllowed 并不是一个不可预测的情况，但我在你的代码中看到了 ForbiddenException。 handleRequest 被创建来做一些有用的事情，当我们无法完成时，抛出异常是很正常的。对我来说，sendErrorCode 看起来像是向前端抛出异常。 - Serhii Shliakhov

@matchish "如果需要记录错误怎么办？" 对于这段代码来说，!isAllowed并不一定是一个错误/异常（它是一个已处理的用例），因此无需记录它。但是现在，我们正在讨论我们不知道其底层语义和用例的代码，并且它与此Q&A的原始目的相去甚远。 - James Thorpe

我说的不是日志记录，它可以是其他任何东西。如果你不仅需要发送错误代码，那么你将需要更改两个代码片段。在我看来，这并不是DRY。 - Serhii Shliakhov

@matchish，将错误日志记录在专门处理错误代码（sendErrorCode）中，相比将错误日志记录在处理请求的代码中，更违反SRP原则吗？ - Old Pro

Answer 6

我认为 James Thorpe 的答案有一个缺点，就是不符合 DRY 原则。在调用 sendError 时，两种情况下都要处理异常。假设我们有许多代码行具有类似的逻辑，可能会抛出异常。我认为可以更好地解决这个问题。

以下是我的解决方案：

async function doSomethingOnAllowedRequest(req) {
    let isAllowed = await checkIfIsAllowed(req);
    if (!isAllowed) {
       throw new ForbiddenException("You're not allowed to do that.");
    }
    doSomething(req);
}
static async handleRequest(req) {
    try {
        let result = await doSomethingOnAllowedRequest(req);
        sendResult(result);
    } catch(err) {
        sendErrorCode(err);
    }
}

- matchish

---

它是DRY的，如果您不被允许执行操作，则会发送非常特定的错误消息。catch用于所有其他异常。在我看来，您只是将“不要使用异常进行流程逻辑控制”移动到另一个方法中。与不抛出任何异常相比，抛出异常并不便宜（在计算上）-如果可以避免这样做，最好不要这样做。 - James Thorpe

我更注重可读性而非性能。 在我看来，这不仅仅是移动。doSomething也可能会抛出异常，但我认为在这种情况下我们不能说我们使用异常来控制流程。 如果请求被允许，你可以执行doSomething，如果不允许，则应该抛出异常。 - Serhii Shliakhov

使用try...catch...finally怎么样？我曾经使用这个语句在finally块中释放一些资源，而且try块中的几个调用也可能会抛出异常。如果我必须在本地中断try流并释放这些资源，我想没有比在那里抛出更易读的解决方案了。 - David

Answer 7

我不同意最被赞同的答案。

理由如下：

1. 你知道它为什么失败；在try-catch块内抛出异常没有必要。你不需要向堆栈跟踪中添加噪声。

2. 你简化了编写单元测试的过程。您不需要在单元测试中捕捉异常以测试分支。

3. throw语句的开销很大，因为它需要生成堆栈跟踪，所以在性能方面不足够好。

- Patrice Thimothee

Answer 8

根据《Clean Code》中Uncle Bob的建议，一个函数应该只做一件事情，并且做好它。如果在你的函数中出现了抛出并在本地捕获异常的情况，很可能你的函数不仅仅是在做一件事情，这会使得它难以阅读和修改。

我建议您重新审视您的函数，并将抛出异常的部分提取到外部函数中。以您的情况为例，我会将其重写为：

static async handleRequest(req) {
  try {
    if (isAllowed(req)) {
      let result = await doSomething(req); // can also raise exceptions
      sendResult(result);
    }
  } catch(err) {
    sendErrorCode(err);
  }
}

static async isAllowed(req) {
  let isAllowed = await checkIfIsAllowed(req);
  if (!isAllowed) {
    throw new ForbiddenException("You're not allowed to do that.");
  }
  return isAllowed;
}

- Pasquale

Answer 9

这可能会给你一些提示，也许可以成为原因（不确定是否相关）。 捕获语句无法捕获错误

你的 try catch 块未能成功的原因是 Ajax 请求是异步的。Try catch 块将在 Ajax 调用和发送请求本身之前执行，但错误是在稍后返回结果时抛出的。

当执行 try catch 块时，没有错误；当抛出错误时，就没有 try catch。如果您需要针对 Ajax 请求使用 try catch，请始终将 Ajax 的 try catch 块放置在 success 回调内部，而不是外部。

- Vladyslav Didenko