0

I have a simple Web app running with NodeJS and Express. It has a route where an outside 3rd party can POST us a XML document, which we then convert to JSON then save to our MongoDB database. A few things can go wrong:

The XML can be malformed

The request might be empty

The outside 3rd party might send us duplicate documents

Rather than having an endless series of then() blocks, going deeper and deeper, indented further and further, I wanted to throw an exception for each possible error, and then catch those errors at the top level and process them there.

So we find a unique id and then check to see if this unique id is already in MongoDB:

// will throw an error if there is a duplicate
document_is_redundant(AMS_945, unique_id);

The function looks like this:

function document_is_redundant(this_model, this_unique_id) {

return this_model.findOne({ unique_id : this_unique_id })
.exec()
.then((found_document) => {
    // 2021-11-28 -- if we find a duplicate, we throw an error and handle it at the end
    // But remember, we want to return a HTTP status code 200 to AMS, so they will stop
    // re-sending this XML document.
    if (found_document != 'null') {
    throw new DocumentIsRedundantException(this_unique_id);
    }
});
// no catch() block because we want the exception to go to the top level
}

This gives me: UnhandledPromiseRejectionWarning

Maybe I'm thinking too much like Java instead of Javascript, but I was assuming that if I didn't catch() the exception in that function, it would bubble up to the top level, which is where I want to deal with it. Also assumed it would interrupt the flow of the code at the line where I call the function.

Sadly, the uncaught exception does not interrupt the main thread of execution, so the document is saved, even when it is a duplicate.

So I'm left thinking the only way I can make this work is to return the Promise from the function and then have a then() block after the call to the document_is_duplicate function?

I dislike having to nest then() blocks inside of then() blocks, several levels deep. This seems like bad code. Is there another way?

charlottesville
  • 467
  • 5
  • 19
  • `Rather than having an endless series of then() blocks, going deeper and deeper, indented further and further` you really want to drop all the `.then()` literature, and adopt the `async/await` syntax :) – Jeremy Thille Nov 29 '21 at 11:05
  • Would I then be able to handle exceptions as I here assumed? – charlottesville Nov 29 '21 at 11:06
  • Ha, no, that's the only problem. `async/await` is awesome, it's clean, lean, easy to write, read, debug and maintain, gets you rid of all the `.then().then().then()` callback hell and asynchronous nightmare, but there's ONE thing you can't do with it, is catch an error :( Kidding, _of course_ you can... Instead of `.catch(err)` use `try{ } catch(err) { }`, and if you want your exception to go up one level, just don't catch it. Job done. – Jeremy Thille Nov 29 '21 at 11:10
  • If you'd like to post an example of bubbling an exception up to the top level in a scenario that uses async/await then I'll vote you as the person who answered the question. – charlottesville Nov 29 '21 at 11:12
  • "*Rather than having an endless series of then() blocks, going deeper and deeper, indented further and further*" read [Aren't promises just callbacks?](https://stackoverflow.com/q/22539815) because you're doing it wrong. – VLAZ Nov 29 '21 at 11:18
  • "*The only way I can make this work is to return the Promise from the function*" - yes, you already do that. "*and then have a then() block after the call to the `document_is_duplicate` function?*" - yes, or an `await` operator. You must use one of the two to handle the promise. – Bergi Nov 29 '21 at 13:23
  • Jeremy Thille, "if you want your exception to go up one level, just don't catch it." I tried exactly this and got UnhandledPromiseRejectionWarning. – charlottesville Nov 30 '21 at 05:28

1 Answers1

1

Not sure why you want to throw an error if your document exists. Look for it, Mongoose will return a document if it exists, or null if it does not. Then simply await the result. Mongoose methods can be awaited, and if you add .exec() they even return a true Promise, which makes your life even easier :

const document_is_redundant = (this_model, unique_id) => this_model.findOne({ unique_id }).lean().exec();

// Now you use it this way
if( !(await document_is_redundant(AMS_945, unique_id))){ // If the returned value is not null
  console.log("Document is redundant! Aborting")
  return;
}
// Returned value was null
console.log("The document doesn't exist yet!")
Jeremy Thille
  • 26,047
  • 12
  • 43
  • 63
  • But await can only be used inside an async function, so I'd have to make all of my functions async if I wanted to use await? – charlottesville Nov 29 '21 at 11:22
  • Also, I'd like to throw different custom Exceptions for each thing that can go wrong, and then I'd like for them to bubble to the top level, where I can determine which HTTP status code I should send, based on the type of error. I can't set the HTTP status code in the functions, as they might resolve in any order, and that would leave me possibly sending a response first and then the status code later, which of course doesn't work. – charlottesville Nov 29 '21 at 11:25
  • @charlottesville yes. It's impossible to handle async result as synchronous code, hence you need `async`/`await` or `.then()`/`.catch()` from the async call point upwards the call chain. Depending on how far up you want to use the result. If you do a "fire and forget" at some point then you don't need more async handling up the call chain. – VLAZ Nov 29 '21 at 11:25
  • In other words, the only way I can know which HTTP status code I'm supposed to send is if I can get all errors to bubble to the top level, though I realize this might be a Java way of thinking and perhaps it doesn't apply so much in Javascript. – charlottesville Nov 29 '21 at 11:26
  • 1
    `await can only be used inside an async function` not since Node v14.8. https://www.stefanjudis.com/today-i-learned/top-level-await-is-available-in-node-js-modules/ – Jeremy Thille Nov 29 '21 at 11:43
  • Which HTTP code you're supposed to send: if the document is not found, send a 404. – Jeremy Thille Nov 29 '21 at 11:44
  • Jeremy Thille, no, you don't understand the situation. I'm interacting with a 3rd party API and I have to follow their documentation in terms of what HTTP status codes to send. 500s and 400s and timeouts should trigger them to re-send the document. "Send a message exactly once" is currently one of the unsolved problems of computer science. Assuming they will send a message multiple times, I try to screen out duplicates, but here I send a 200 to let them know they should stop sending that message. If the document is not found, I also send a 200, as that is the happy situation -- no duplicate. – charlottesville Nov 30 '21 at 06:41