-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
onunhandledrejection for globalThis? #7013
Comments
We should have it, it is part of the HTML Living Standard: https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onunhandledrejection |
Maybe I'm testing for it wrong? Here's what I tried: globalThis.onunhandledrejection = function() {
console.error("Unhandled rejection");
};
new Promise(() => {
throw new Error("Error");
}); |
I think @kitsonk means we should add it, not we should already have it :-) |
@lucacasonato oh gotcha 😆 |
@lucacasonato Wouldn't this mean that if added, we will now have the option to stop Deno from dying in uncaught exceptions? |
Uncaught exceptions are not the same as uncaught rejections. This is for when you don't define a .catch on a promise. |
Got it, didn't notice it was "rejection", Thanks |
@lucacasonato I think uncaught rejections absolutely fall under "uncaught errors".
Let's say we tried to mirror the browser behaviour by not exiting on a rejection if and only if there is an Nevertheless, I'm against this being usable to prevent exits on rejections. I think we are still breaking our rule at the language level and this is a foot-gun. Note that browsers don't kill the tab on error either way, they only use this to decide whether or not to output the error to the console. So it's not so simple to draw an analogy. |
Is there any update/eta on this? This is an absolutely crucial feature to have to prevent crashes. |
@Skillz4Killz This will not be implemented to prevent crashes. If implemented it will only be able to be used to perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc). It will not be usable to prevent shutdown on an unhandled rejection. Instead you should catch promise rejections correctly. Deno will always die on unhandled exceptions / rejections. |
I'd like to request the ability to change this for uncaught promises. I know a lot of people just say hey if this happens it is your fault. Technically your right. But this is reality. And people make mistakes. Production crashing because of a mistake is a serious issue. Especially when it is so easily preventable. But the problem really gets amplified when even if you are perfect, and your code catches everything and everyone, you can still crash if one of your deps forgets. So you can not just rely on yourself being perfect but the entirety of the world needing to be perfect. This in my opinion makes Deno very hard to want to use in production. Processes crashing are not an acceptable solution imo. Let's take a look at a simple example. Please bare with me here. In this example, what happens is the process crashes. There is no way for me to catch this error. It's literally impossible. Since If this was nested some 10-15 levels deep, we would never be able to debug this and even attempt a fix. The other day, this actually caused my own project to fail. When I saw this, I was confused out of my mind. What on earth is core.js? I don't have any file called core.js. I don't know of any dep that has this and it sounds like it could literally come from any dep or even Deno itself or even V8. I just don't know. Luckily, my project has very little deps so I went through debugging spendings hours to figure this out. Just to find out there was a bug in listenDatagram(which isn't an issue, it's still unstable). The issue is that this behavior can cause a process in production to crash unknowingly. It becomes a nightmare to debug and solve. This is all to say that Deno in production is really hard to vouch for. Realistically Deno is only usable in production if you do not use any deps for your projects. Now for another point of view. Deno is realllllllly hard to use for beginners. A lot of users wanting to use my library are first time developers. This means they are just getting into their very first coding project EVER! I have made it as easy as possible for users like this to be able to get started. They can use the strictest TS settings without even needing to know a thing about typing or casting or anything. However, this 1 issue of them forgetting .catch is a serious issue. Time and time again I have users reporting their processes crashing because they simply forgot to catch something. Not have a training wheel so to speak makes this really difficult. A simple handler to prevent crashes could be soooo useful because I could have this internally built for them so it alerts them of where they are forgetting to catch so that they can fix their code and prevent their processes from crashing. |
I came across this issue because I had a similar cryptic error. Here are my two cents on this. I myself forgot to await a call to this was my error:
How the hell am I supposed to debug that... I'm honestly surprised I was able to find the source. If that was in a library, quoting @Skillz4Killz
This is an oversight that we as developers can't even deal with! MADNESS!
I completely agree with @Skillz4Killz here, the error output is not helpful at all with debugging. Just a better stack trace would be enough to help developers debug their code. The linter didn't tell me that I had an async call without a
Yup. And if it keeps going like this it's only going to get worse. Everybody makes mistakes. At least make them possible to debug. |
My use case is a testing tool, which is supposed to detect and report any, even unexpected exceptions, raised by the test code. This is not possible if the process synchronously dies on any exception. |
My use case is a tool that serves as a container for running multiple 3rd-party web handler scripts. I need to be able to keep the server running (and capture and report the error to the user myself) if one of the scripts forgets to Panicking on every unhandled rejection is certainly a reasonable default, but should be configurable either when starting the |
I'm just trying to figure out how to restart my deno app when it does crash. Currently I don't see anyway of doing that short of adding node module pm2 (yuck!) |
I believe processes, in general, should not be responsible for restarting themselves. That's the responsibility of the underlying platform the processes are running at (see container orchestration or process managers). Similarly, in response to other fellows, process crashing in production when there are unhandled exceptions is the safest bet. Since the process should be automatically restarted by its manager and restarting should be the best way to ensure the process is properly initialized again. |
Doing something on unhandled rejection/error is the valid use case this issue seems to be about. I don't know the specifics of your project, but at first glance, I'd suggest running those scripts as separate processes. So when they crash your "container" stays alive and deals with the other processes bug as you please. |
I will implement this API as it's required for Node compatiblity mode. |
Provide a programmatic means of intercepting rejected promises without a .catch() handler. Needed for Node compat mode. Also do a first pass at uncaughtException support because they're closely intertwined in Node. It's like that Frank Sinatra song: you can't have one without the other. Stepping stone for denoland#7013.
Provide a programmatic means of intercepting rejected promises without a .catch() handler. Needed for Node compat mode. Also do a first pass at uncaughtException support because they're closely intertwined in Node. It's like that Frank Sinatra song: you can't have one without the other. Stepping stone for denoland#7013.
Provide a programmatic means of intercepting rejected promises without a .catch() handler. Needed for Node compat mode. Also do a first pass at uncaughtException support because they're closely intertwined in Node. It's like that Frank Sinatra song: you can't have one without the other. Stepping stone for #7013.
Note to self:
|
Landed in #12994, it will be released in v1.24 on July, 20th. |
Did this get reverted and not make it into the release? |
It was released in v1.24 https://deno.com/blog/v1.24#unhandledrejection-event |
Thanks! I couldn't find the relevant commit after the revert but there it is in the release notes! |
I was looking for an equivalent to the Node.js "unhandledrejection" event in Deno, but it appears there isn't one on
globalThis
.There does appear to be an
onunhandledrejection
event handler defined on workers, though.Is there an equivalent event outside of workers, and if not, could there be? 😄
The text was updated successfully, but these errors were encountered: