FindOne=>save()组合无法正常工作问题排查

Hey there, let's figure out why your findOne -> save flow isn't working as expected. I see a few key issues in your code that are causing this, and we'll fix them step by step:

1. You're redirecting before async operations finish #

Node.js runs asynchronous operations in the background, and your res.redirect('/admin') is sitting outside the findOne and save callbacks. That means the server sends the redirect response before the database lookup or save completes—so your changes never get a chance to persist to the database. You need to move the redirect inside the save callback, so it only runs once the update is successful.

2. No check for missing documents #

If Banner.findOne() doesn't find a document matching the _id from the URL, doc will be null. Trying to update properties like doc.time on a null value will throw an error, but your try/catch block won't catch it—because async callback errors don't bubble up to the surrounding synchronous try/catch. You need to add a check to make sure doc exists before modifying it.

3. Incomplete error handling #

Right now you're just logging errors to the console, but you're not passing them to your error-handling middleware or notifying the client that something went wrong. This makes it hard to debug issues and leaves users in the dark if the update fails.

Fixed Code (Callback Style) #

Here's your code with all these issues addressed:

router.route('/banner/:_id')
.post((req, res, next) => {
  console.log('got here');
  const r = req.body;
  const bannerId = req.params._id;

  Banner.findOne({ _id: bannerId }, (findErr, doc) => {
    // Handle lookup errors
    if (findErr) {
      console.error('Failed to find banner:', findErr);
      return next(findErr); // Pass error to your error middleware
    }

    // Handle missing banner
    if (!doc) {
      const notFoundError = new Error(`Banner with ID ${bannerId} doesn't exist`);
      notFoundError.status = 404;
      return next(notFoundError);
    }

    // Update banner properties
    doc.time = r.time;
    doc.date = r.date;
    doc.technicalIssue = r.technicalIssue;
    doc.message = r.message;

    // Save changes and redirect only on success
    doc.save((saveErr) => {
      if (saveErr) {
        console.error('Failed to save banner:', saveErr);
        return next(saveErr);
      }
      console.log('Banner updated successfully');
      res.redirect('/admin');
    });
  });
});

Even Better: Async/Await Version #

For cleaner, more readable code (and easier error handling), use async/await instead of nested callbacks:

router.route('/banner/:_id')
.post(async (req, res, next) => {
  try {
    console.log('got here');
    const r = req.body;
    const bannerId = req.params._id;

    // Find the banner
    const doc = await Banner.findOne({ _id: bannerId });
    if (!doc) {
      const notFoundError = new Error(`Banner with ID ${bannerId} doesn't exist`);
      notFoundError.status = 404;
      throw notFoundError;
    }

    // Update properties
    doc.time = r.time;
    doc.date = r.date;
    doc.technicalIssue = r.technicalIssue;
    doc.message = r.message;

    // Save changes
    await doc.save();
    console.log('Banner updated successfully');
    res.redirect('/admin');
  } catch (err) {
    console.error('Error updating banner:', err);
    next(err); // Pass error to middleware
  }
});

Key Takeaways #

• Always wait for async database operations to finish before sending a response (like redirecting)
• Handle edge cases like missing documents to avoid runtime errors
• Pass errors to your error-handling middleware instead of just logging them—this helps with debugging and proper client feedback
• Async/await makes asynchronous code much easier to read and maintain compared to nested callbacks

内容的提问来源于stack exchange，提问作者Alex Ironside