Awaiting unresolvable promise should throw instead of Node terminating the process.

[masbicudo]

Version

v17.9.0

Platform

Microsoft Windows NT 10.0.22000.0 x64

Subsystem

No response

What steps will reproduce the bug?

const p = new Promise((resolve, reject) => { })
try {
    await p
}
catch {
    console.log("unresolvable promise detected")
}

How often does it reproduce? Is there a required condition?

Always. No required condition.

What is the expected behavior?

This will crash the execution. Instead, shouldn't this be an exception?
If this can be detected to allow NodeJS to end execution, otherwise it would await forever, why not make it an exception to allow Javascript programs detect this condition, on the await promise statement.

What do you see instead?

NodeJS just stops executing and exists without any possibility of control from the executing script.

Additional information

No response

[masbicudo]

If this is by design, then I propose a new feature:

• a method that allows each promise to decide what to do when it is impossible to resolve/reject it.

Example 1:

const p = new Promise(() => {})
p.on("unresolvable", (resolve, reject, defaultAction) => {
    exit(13) // same as default behavior (but now I have a choice!)
})
await p // exit NodeJS process, with the same error code it would have otherwise

Example 2:

const p = new Promise(() => {})
p.on("unresolvable", (resolve, reject, defaultAction) => {
    defaultAction() // default behavior (but now I have a choice!)
})
await p // exit NodeJS process, with the same error code it would have otherwise

Example 3:

const p = new Promise(() => {})
p.on("unresolvable", (resolve, reject, defaultAction) => {
    reject(new Error("unresolvable"))
})
await p // this will throw the given `new Error("unresolvable")`

Example 4:

const p = new Promise(() => {})
p.on("unresolvable", (resolve, reject, defaultAction) => {
    resolve(1)
})
await p // this will resolve to the value `1`

[masbicudo]

Why I need this?

To make the following work to the end.

When using read_file_lines function bellow, with a file that is not empty, this works, and it is very nice to use async generators.
It is a valid usage, unless when the file is empty. When that happens, the callback is never called, and the promise becomes unresolvable. NodeJS detects that on the usage of await and terminates the process with exit code 13. But I sincerely think that the choice on what to do, should be delegated to the user script. I could ask the author of line-reader, to change it, or help me change it... but look, solving the problem is not my only intention, since this proposal could be considered an improvement.

import fs from 'fs'
import lineReader from 'line-reader'

function create_puppet_promise() {
    let puppet_promise = {}
    puppet_promise.promise = new Promise((resolve, reject) => {
        puppet_promise.resolve = resolve
        puppet_promise.reject = reject
    })
    return puppet_promise
}

async function* read_file_lines(file_path) {
    let first_puppet_promise = create_puppet_promise()
    let last_puppet_promise = first_puppet_promise
    lineReader.eachLine(file_path, (line, last) => {
        const next_puppet_promise = last ? null : create_puppet_promise()
        const data = { line, last, }
        const resolve_data = { data, next_puppet_promise, }
        last_puppet_promise.resolve(resolve_data)
        last_puppet_promise = next_puppet_promise
    })
    async function next_value() {
        let item = await first_puppet_promise.promise
        first_puppet_promise = item.next_puppet_promise
        return item.data
    }
    while (true) {
        const p = next_value()
        let data
        data = await p
        yield data.line
        if (data.last) break
    }
}

export async function display_file_lines_async(lines_async_gen) {
    for await (const line of lines_async_gen) {
        console.log(line)
    }
}


// preparing example files
fs.writeFileSync("non-empty-file", "1")
fs.writeFileSync("empty-file", "")


// This will work!!! =)
await display_file_lines_async(read_file_lines("non-empty-file"))

// This does not work... and crashes NodeJS =(
await display_file_lines_async(read_file_lines("empty-file"))

// This is unreachable, since NodeJS terminates forcibly with code 13
console.log("end")

By making these changes:

function create_puppet_promise() {
    let puppet_promise = {}
    puppet_promise.promise = new Promise((resolve, reject) => {
        puppet_promise.resolve = resolve
        puppet_promise.reject = reject
    })
    puppet_promise.promise.on(
            "unresolvable",
            (resolve, reject, defaultAction) => resolve({ line: "", last: true, }
        )
    return puppet_promise
}

[Trott]

Very incomplete list of previous content related to this:

• Process exits with unresolved promises promises-debugging#16
• Nodejs does not wait for promise resolution - exits instead #22088
• https://stackoverflow.com/questions/46914025/node-exits-without-error-and-doesnt-await-promise-event-callback
• https://stackoverflow.com/questions/46966890/what-happens-when-a-promise-never-resolves
• https://www.youtube.com/watch?v=PNa9OMajw9w
• Promise.all() dies silently if no resolve #29355
• Node stopping execution prematurely even though promise is still pending. #25504

then I propose a new feature:

FWIW, you could do this now by keeping track of your promises, listening for the 'exit' event, and checking their status at that time.

@nodejs/promises-debugging

[aduh95]

Duplicate of #42868?

[masbicudo]

Thank you @Trott and @aduh95 for pointing relevant related links, and for taking the time to respond. I have been looking at them since yesterday, and learnt a lot.

@Trott Thank you very much for pointing out that I could solve this using process 'exit' event.

With the process 'exit' event I could rewrite my create_puppet_promise function.
It now returns an object that have the events I have proposed previously.
Now I can do something like this:

const pp = create_puppet_promise()
    .on("unresolved", (resolve, reject) => resolve("some default value"))

const v = await pp.promise

console.log(`ended with v = ${v}, state = ${pp.state}`)

Now the create_puppet_promise looks like this:

function create_puppet_promise() {
    let resolve, reject
    let state = "pending"
    let handlers = {
        "unresolved": [],
        "resolving": [(val) => state = "resolved"],
        "rejecting": [(val) => state = "rejected"],
    }
    const onExit = () => {
        if (state === "pending")
            for (const cb of handlers["unresolved"])
                cb(resolve, reject)
    }
    const removeOnExit = process.removeListener.bind(process, "exit", onExit)
    const onCallDo = (f, cbs) => (...args) => {
        for (const cb of cbs)
            cb(...args)
        return f(...args)
    }
    const promise = new Promise((s, j) => {
        resolve = onCallDo(s, handlers["resolving"])
        reject = onCallDo(j, handlers["rejecting"])
    })
    const puppet = {
        promise, resolve, reject,
        on(event_name, callback) {
            if (event_name == "unresolved") {
                process.on("exit", onExit)
                handlers["resolving"].push(removeOnExit)
                handlers["rejecting"].push(removeOnExit)
            }
            handlers[event_name].push(callback)
            return this
        },
        once(event_name, callback) {
            const otherCallback = (...args) => {
                this.removeListener(otherCallback)
                return callback(...args)
            }
            otherCallback.decorated = callback
            return this.on(event_name, otherCallback)
        },
        removeListener(event_name, callback) {
            const evt_handler = handlers[event_name]
            for (let it = 0; it < evt_handler.length; it++)
                if (evt_handler[it].decorated === callback)
                    evt_handler[it] = callback
            evt_handler.remove(callback)
            return this
        },
        get state() {
            return state
        }
    }
    return puppet
}

In the end, this makes more sense. These promise I need must be controlled externally, that is why they have their resolve and reject callbacks leaked from the encapsulation of the promise. It makes sense to have any special behavior implemented specifically for these, and not all of the existing promises.

If I couldn't make any better to NodeJs, at least now I have better understanding of it. I hope this code helps someone one day.

I think this is a "case closed" for this issue. Again, thanks a lot!

[masbicudo]

Oh, I also rewrote the function to read one line at a time from a file, using an async generator.
In the end, removing the dependency on the external library ended up clearing the need for the extra promise events that I needed, though I still need the promise callbacks (resolve cb at least) to leak from the promise.

Here is the code:

const read_file_lines_default_options = {
    buffer_length: 10240,
    ignore_empty_last_line: false,
}

async function* read_file_lines(
    file_path,
    options = read_file_lines_default_options
) {
    options = Object.assign(
        {},
        read_file_lines_default_options,
        options,
    )
    let first_puppet_promise = create_puppet_promise()
    let last_puppet_promise = first_puppet_promise
    const reader = fs.createReadStream(
        file_path,
        {
            highWaterMark: options.buffer_length,
        })
    let close_line = null
    reader.on("end", () => {
        // console.log("event: end")
        if (!options.ignore_empty_last_line)
            close_line = ""
    })
    const line_reader = readline.createInterface({
        input: reader
    })
    const handler = (event, line) => {
        // console.log("event: " + event)
        const next_puppet_promise =
            line === null
                ? null
                : create_puppet_promise()
        last_puppet_promise.resolve(
            {
                line: line ?? close_line,
                next_puppet_promise,
            })
        last_puppet_promise = next_puppet_promise
        close_line = null
    }
    line_reader.on("line", handler.bind(this, "line"))
    line_reader.on("close", handler.bind(this, "close", null))
    async function next_value() {
        if (first_puppet_promise == null)
            return null
        const p = first_puppet_promise.promise
        const item = await p
        first_puppet_promise = item.next_puppet_promise
        return item.line
    }
    while (true) {
        const line = await next_value()
        if (line === null) break
        yield line
    }
}

[aduh95]

In case you don't know that already, the readline.Interface is itself an async generator, so you may find it useful to simplify your code:

async function* read_file_lines(
    file_path,
    options = undefined
) {
    options = Object.assign(
        {},
        read_file_lines_default_options,
        options,
    )
    const reader = fs.createReadStream(
        file_path,
        {
            highWaterMark: options.buffer_length,
        })
    const line_reader = readline.createInterface({
        input: reader,
        crlfDelay: Infinity,
    })
    yield* line_reader
    if (!options.ignore_empty_last_line)
            yield ""
}