From ec02bbd530a87eda0a6e6d1649dda5d951acdd0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Wed, 14 Sep 2022 19:37:49 +0200 Subject: [PATCH 01/21] Create exception-types.md --- text/exception-types.md | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 text/exception-types.md diff --git a/text/exception-types.md b/text/exception-types.md new file mode 100644 index 00000000..d09669f2 --- /dev/null +++ b/text/exception-types.md @@ -0,0 +1,44 @@ +* Start Date: 2022-09-14 +* RFC Type: feature +* RFC PR: - +* RFC Status: draft + +# Summary + +This RFC suggests a feature which introduces additional types of exceptions next to `unhandled`. + +# Motivation + +Currently, exception which cause the running software to exit are marked as `unhandled: true`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. + +Sentry shows exceptions which aren't caught but also do not cause the software to exit in the same way as exceptions which are manually caught by a developer. This seems rather unintuitive and makes exceptions seem less severe than they are. + +This issue impacts, for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. + +Another issue is, that excpetions which don't cause the application to exit but are uncaught, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). + +# Proposal + +Based on the problem stated above, I propose to introduce the types of `manually caught`, `uncaught`, `software crashed` (this is the same as the current `unhandeld`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: + +- `manually caught`: The exception was recorded by a developer. May or may not be visually indicated by the Sentry user interface. +- `uncaught`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception however didn't cause the software to quite, and the software will continue to be executed. This should be visualized in the Sentry user interface. +- `software crashed`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. + +This enables the consideration in the `session health` metric. + +I'm guessing this affects data ingestion layer, but since I'm not familiar with that part, I can't comment on the impact theses changes would have on that. + +# Other options considered + +Mark both `uncaught` and `software crashed` as the same type. +That would however make it impossible to differentiate between those exception types in the `session health` metric. + +# Approaches taken by other monitoring tools + +- Crashlytics does not differentiate between `uncaught` and `software crashed`. + +# Unresolved Questions + +- Which SDKs would profit from this RFC? Is it the majority, a good chunk of it, or just the minory? +- Are there any other exception types next to the ones metioned in this RFC? From 3505f7c102947bdabb858cbca12355d088785e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Thu, 15 Sep 2022 09:51:21 +0200 Subject: [PATCH 02/21] Update exception-types.md --- text/exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/exception-types.md b/text/exception-types.md index d09669f2..1637efc9 100644 --- a/text/exception-types.md +++ b/text/exception-types.md @@ -23,7 +23,7 @@ Based on the problem stated above, I propose to introduce the types of `manually - `manually caught`: The exception was recorded by a developer. May or may not be visually indicated by the Sentry user interface. - `uncaught`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception however didn't cause the software to quite, and the software will continue to be executed. This should be visualized in the Sentry user interface. -- `software crashed`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. +- `software crashed`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the unhandled flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). This enables the consideration in the `session health` metric. From 34a49404bd5516aa9e587c840d1cf5be8f5ec24c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Thu, 15 Sep 2022 18:19:17 +0200 Subject: [PATCH 03/21] Update exception-types.md --- text/exception-types.md | 47 ++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/text/exception-types.md b/text/exception-types.md index 1637efc9..451fc573 100644 --- a/text/exception-types.md +++ b/text/exception-types.md @@ -13,30 +13,61 @@ Currently, exception which cause the running software to exit are marked as `unh Sentry shows exceptions which aren't caught but also do not cause the software to exit in the same way as exceptions which are manually caught by a developer. This seems rather unintuitive and makes exceptions seem less severe than they are. -This issue impacts, for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. +This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. Another issue is, that excpetions which don't cause the application to exit but are uncaught, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). # Proposal -Based on the problem stated above, I propose to introduce the types of `manually caught`, `uncaught`, `software crashed` (this is the same as the current `unhandeld`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: +Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `unhandeld`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: -- `manually caught`: The exception was recorded by a developer. May or may not be visually indicated by the Sentry user interface. -- `uncaught`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception however didn't cause the software to quite, and the software will continue to be executed. This should be visualized in the Sentry user interface. -- `software crashed`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the unhandled flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). +- `handled`: The exception was recorded by a developer via `Sentry.capture*` method. May or may not be visually indicated by the Sentry user interface. +- `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface. +- `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `unhandled` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). -This enables the consideration in the `session health` metric. +In order to propagate those exception types, the exception mechanism needs to be adapted: + +```json +{ + "exception": { + "values": [ + { + "type": "Error", + "value": "An error occurred", + "mechanism": { + "type": "generic", + "handled": true, + "process_terminated": false // <--- newly introduced field + } + } + ] + } +} +``` + +In order to achieve backwards compatibility, in the absence of the `process_termination` flag, the current behavior stays as is. +As soon as the `process_terminated` flag is present the bavior is as follows: + +- `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception +- `handled = true` and `process_terminated = false`: Exception is not handled by the user but didn't cause the software to terminate. Same as `unhandled` in the list above +- `handled = false` and `process_terminated = true`: Software terminated after an unhandled exception. Same as `process termination` in the list above +- `handled = true` and `process_terminated = false`: Exception was reported via `Sentry.capture*()` method. Same as `handled` in the list above. + +In the absence of the `handled` or its value being null, it's assumed to be `handled = true`. + + +The introduction of the `process_terminated` flag enables the consideration of such exception types in the `session health` metric. I'm guessing this affects data ingestion layer, but since I'm not familiar with that part, I can't comment on the impact theses changes would have on that. # Other options considered -Mark both `uncaught` and `software crashed` as the same type. +Unhandled exceptions, which don't cause a process termination, are considered like exceptions which cause the process to terminate. That would however make it impossible to differentiate between those exception types in the `session health` metric. # Approaches taken by other monitoring tools -- Crashlytics does not differentiate between `uncaught` and `software crashed`. +- Crashlytics just differentiates between manually caught exceptions and unhandled exceptions, regardless of wether they cause the process to terminate. # Unresolved Questions From 95844a01cb3c1590fc59f3843e3b46279a2e7142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sat, 17 Sep 2022 20:17:45 +0200 Subject: [PATCH 04/21] Rename exception-types.md to 0010-exception-types.md --- text/{exception-types.md => 0010-exception-types.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename text/{exception-types.md => 0010-exception-types.md} (100%) diff --git a/text/exception-types.md b/text/0010-exception-types.md similarity index 100% rename from text/exception-types.md rename to text/0010-exception-types.md From 242e29ca8560c302996d896c96582bae6c847d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sat, 17 Sep 2022 20:18:03 +0200 Subject: [PATCH 05/21] Update 0010-exception-types.md --- text/0010-exception-types.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 451fc573..d2707a82 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -1,6 +1,6 @@ -* Start Date: 2022-09-14 +* Start Date: 2022-09-17 * RFC Type: feature -* RFC PR: - +* RFC PR: 0010 * RFC Status: draft # Summary From 6659ed31b3edb5b4841c33a366f9869338142edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:36:31 +0200 Subject: [PATCH 06/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index d2707a82..638feb0e 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -5,7 +5,7 @@ # Summary -This RFC suggests a feature which introduces additional types of exceptions next to `unhandled`. +This RFC suggests a feature which introduces additional types of exceptions next to `mechanism.handled`. # Motivation From 593abcd570c2ce3e16f6ea989cd7e2fc45c1b7cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:36:39 +0200 Subject: [PATCH 07/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 638feb0e..58705262 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -9,7 +9,7 @@ This RFC suggests a feature which introduces additional types of exceptions next # Motivation -Currently, exception which cause the running software to exit are marked as `unhandled: true`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. +Currently, exception which cause the running software to exit are marked as `handled: false`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. Sentry shows exceptions which aren't caught but also do not cause the software to exit in the same way as exceptions which are manually caught by a developer. This seems rather unintuitive and makes exceptions seem less severe than they are. From 6945acdd0c01ec3e1fdb559d0e47d3d250ca1288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:36:56 +0200 Subject: [PATCH 08/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 58705262..a7de6bfd 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -11,7 +11,7 @@ This RFC suggests a feature which introduces additional types of exceptions next Currently, exception which cause the running software to exit are marked as `handled: false`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. -Sentry shows exceptions which aren't caught but also do not cause the software to exit in the same way as exceptions which are manually caught by a developer. This seems rather unintuitive and makes exceptions seem less severe than they are. +Sentry shows exceptions which aren't caught by the developer (unhandled) but also do not cause the software to exit in the same way as exceptions which are manually caught by the developer. This seems rather unintuitive and makes exceptions seem less severe than they are. This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. From b6d2275086a1083c2a05bba233dbad2546762ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:37:10 +0200 Subject: [PATCH 09/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index a7de6bfd..dd21d18e 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -15,7 +15,8 @@ Sentry shows exceptions which aren't caught by the developer (unhandled) but als This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. -Another issue is, that excpetions which don't cause the application to exit but are uncaught, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). +Another issue is, that excpetions which don't cause the application to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). +Currently, the session would be marked as `errored` instead of `crashed`. # Proposal From 06efd2e0e98e59212e9ed8f486af71e07b12616d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:37:20 +0200 Subject: [PATCH 10/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index dd21d18e..db36e5b7 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -20,7 +20,7 @@ Currently, the session would be marked as `errored` instead of `crashed`. # Proposal -Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `unhandeld`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: +Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `handled`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: - `handled`: The exception was recorded by a developer via `Sentry.capture*` method. May or may not be visually indicated by the Sentry user interface. - `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface. From 49b470aa3ec5f22a343d5cdb9d96dc9bc91913e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:37:28 +0200 Subject: [PATCH 11/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index db36e5b7..e7315b20 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -24,7 +24,7 @@ Based on the problem stated above, I propose to introduce the types of `handled` - `handled`: The exception was recorded by a developer via `Sentry.capture*` method. May or may not be visually indicated by the Sentry user interface. - `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface. -- `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `unhandled` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). +- `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `handled` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). In order to propagate those exception types, the exception mechanism needs to be adapted: From 9c4cc0b6729e01b1d062bc09497ce46a97dedd7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 18 Sep 2022 09:37:40 +0200 Subject: [PATCH 12/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index e7315b20..699dd81b 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -50,7 +50,7 @@ In order to achieve backwards compatibility, in the absence of the `process_term As soon as the `process_terminated` flag is present the bavior is as follows: - `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception -- `handled = true` and `process_terminated = false`: Exception is not handled by the user but didn't cause the software to terminate. Same as `unhandled` in the list above +- `handled = false` and `process_terminated = false`: Exception is not handled by the user but didn't cause the software to terminate. Same as `unhandled` in the list above - `handled = false` and `process_terminated = true`: Software terminated after an unhandled exception. Same as `process termination` in the list above - `handled = true` and `process_terminated = false`: Exception was reported via `Sentry.capture*()` method. Same as `handled` in the list above. From 4da10aac8d84383fff308d2630e4606d48742a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Wed, 21 Sep 2022 18:01:06 +0200 Subject: [PATCH 13/21] Update 0010-exception-types.md --- text/0010-exception-types.md | 55 ++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 699dd81b..d455494c 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -18,7 +18,7 @@ This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsent Another issue is, that excpetions which don't cause the application to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). Currently, the session would be marked as `errored` instead of `crashed`. -# Proposal +# Option 1 Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `handled`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: @@ -26,6 +26,14 @@ Based on the problem stated above, I propose to introduce the types of `handled` - `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface. - `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `handled` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). +A user of Sentry should be able to + +- filter events on the issues page or discover for the newly introduces exception types. +- highlight (similar to the unhandled label) events of the type unhandled and process termination. +- get alerted for events of the type unhandled and process termination separately. + +Currently, there's an unhandled label on the issue's page but it's only highlighted for process termination errors. + In order to propagate those exception types, the exception mechanism needs to be adapted: ```json @@ -49,28 +57,59 @@ In order to propagate those exception types, the exception mechanism needs to be In order to achieve backwards compatibility, in the absence of the `process_termination` flag, the current behavior stays as is. As soon as the `process_terminated` flag is present the bavior is as follows: -- `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception +- `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception. This should never happen and is invalid. - `handled = false` and `process_terminated = false`: Exception is not handled by the user but didn't cause the software to terminate. Same as `unhandled` in the list above - `handled = false` and `process_terminated = true`: Software terminated after an unhandled exception. Same as `process termination` in the list above - `handled = true` and `process_terminated = false`: Exception was reported via `Sentry.capture*()` method. Same as `handled` in the list above. -In the absence of the `handled` or its value being null, it's assumed to be `handled = true`. - +In the absence of the `handled` or its value being null, it's assumed to be `handled = true`. This is also the current behavior. The introduction of the `process_terminated` flag enables the consideration of such exception types in the `session health` metric. -I'm guessing this affects data ingestion layer, but since I'm not familiar with that part, I can't comment on the impact theses changes would have on that. +# Option 2 + +This one is very similar to option 1, however instead of an additional flag, this introduces an enum for the different types. +Once again, the mechanism needs to be adapted: + +```json +{ + "exception": { + "values": [ + { + "type": "Error", + "value": "An error occurred", + "mechanism": { + "type": "generic", + "exception_type": "handled|unhandled|process_termination", // <--- newly introduced field + } + } + ] + } +} +``` + +If the currently available `handled` flag is also present, the `exception_type` flag takes precedence. The `handled` flag however should become deprecated. + +The introduction of the `exception_type` flag enables the consideration of such exception types in the `session health` metric. -# Other options considered +# Option 3 -Unhandled exceptions, which don't cause a process termination, are considered like exceptions which cause the process to terminate. +Unhandled exceptions, which don't cause a process termination, are considered like exceptions which cause the process to terminate and are marked `handled: false` That would however make it impossible to differentiate between those exception types in the `session health` metric. # Approaches taken by other monitoring tools - Crashlytics just differentiates between manually caught exceptions and unhandled exceptions, regardless of wether they cause the process to terminate. +# List of SDK to which this applies + +This list might be incomplete + +- [Flutter](https://github.com/getsentry/sentry-dart/issues/456) +- Browser SDKs +- Unity +- React Natice + # Unresolved Questions -- Which SDKs would profit from this RFC? Is it the majority, a good chunk of it, or just the minory? - Are there any other exception types next to the ones metioned in this RFC? From 9021551c07ccdfeea51597f2c741a2774f9e0cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Fri, 23 Sep 2022 10:20:43 +0200 Subject: [PATCH 14/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index d455494c..039f5af0 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -108,7 +108,7 @@ This list might be incomplete - [Flutter](https://github.com/getsentry/sentry-dart/issues/456) - Browser SDKs - Unity -- React Natice +- React Native # Unresolved Questions From 69ec25012f08e9746b14fd52e25d07c5739a1c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Fri, 23 Sep 2022 10:27:11 +0200 Subject: [PATCH 15/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 039f5af0..2f40069e 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -18,7 +18,7 @@ This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsent Another issue is, that excpetions which don't cause the application to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). Currently, the session would be marked as `errored` instead of `crashed`. -# Option 1 +# Option 1 (recommended) Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `handled`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: From 4c511a731e5960271b70d4f6fd88629961cc0cc2 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Sep 2022 14:06:42 +0200 Subject: [PATCH 16/21] fix --- text/0010-exception-types.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 2f40069e..9fe77828 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -9,13 +9,13 @@ This RFC suggests a feature which introduces additional types of exceptions next # Motivation -Currently, exception which cause the running software to exit are marked as `handled: false`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. +Currently, exception which cause the running software to exit (the process died/hard crash) are marked as `handled: false`. This isn't enough for SDKs where an exception can be unhandled but at the same time doesn't cause the software to exit. Sentry shows exceptions which aren't caught by the developer (unhandled) but also do not cause the software to exit in the same way as exceptions which are manually caught by the developer. This seems rather unintuitive and makes exceptions seem less severe than they are. This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsentry/sentry-dart/issues/456), the Unity SDK, the React Native SDK and possibly more. -Another issue is, that excpetions which don't cause the application to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). +Another issue is, that excpetions which don't cause the software to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). Currently, the session would be marked as `errored` instead of `crashed`. # Option 1 (recommended) @@ -23,16 +23,16 @@ Currently, the session would be marked as `errored` instead of `crashed`. Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `handled`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: - `handled`: The exception was recorded by a developer via `Sentry.capture*` method. May or may not be visually indicated by the Sentry user interface. -- `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface. -- `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `handled` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). +- `unhandled`: Indicates whether the exception was recorded automatically by Sentry through the use of a global exception handler or similar. This exception however didn't cause the software to exit, and the software will continue to be executed. This should be visualized in the Sentry user interface, they have a higher severity than the `handled` ones. +- `process termination`: The exception was recorded automatically by Sentry through the use of a exception handler or similar. The exception caused the software to terminate the execution. This should be visualized in the Sentry user interface. This is currently done by the `handled: false` flag in the [exception mechanism](https://develop.sentry.dev/sdk/event-payloads/exception/#exception-mechanism). A user of Sentry should be able to -- filter events on the issues page or discover for the newly introduces exception types. +- filter events on the issues page or discover for the newly introduces exception types (3 categories). - highlight (similar to the unhandled label) events of the type unhandled and process termination. - get alerted for events of the type unhandled and process termination separately. -Currently, there's an unhandled label on the issue's page but it's only highlighted for process termination errors. +Currently, there's an unhandled label on the issue's page but it's only highlighted for process termination errors (if `handled: false`). In order to propagate those exception types, the exception mechanism needs to be adapted: @@ -54,18 +54,20 @@ In order to propagate those exception types, the exception mechanism needs to be } ``` -In order to achieve backwards compatibility, in the absence of the `process_termination` flag, the current behavior stays as is. +In order to achieve backwards compatibility, in the absence of the `process_terminated` flag, the current behavior stays as is. As soon as the `process_terminated` flag is present the bavior is as follows: -- `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception. This should never happen and is invalid. - `handled = false` and `process_terminated = false`: Exception is not handled by the user but didn't cause the software to terminate. Same as `unhandled` in the list above - `handled = false` and `process_terminated = true`: Software terminated after an unhandled exception. Same as `process termination` in the list above - `handled = true` and `process_terminated = false`: Exception was reported via `Sentry.capture*()` method. Same as `handled` in the list above. +- `handled = true` and `process_terminated = true`: Software was gracefully shut down after an handled exception. This should never happen and is invalid. In the absence of the `handled` or its value being null, it's assumed to be `handled = true`. This is also the current behavior. The introduction of the `process_terminated` flag enables the consideration of such exception types in the `session health` metric. +The [session protocol](https://docs.sentry.io/product/releases/health/#session-status) would need to change as well though, because there are only `errored` and `crashed` states. + # Option 2 This one is very similar to option 1, however instead of an additional flag, this introduces an enum for the different types. @@ -95,7 +97,7 @@ The introduction of the `exception_type` flag enables the consideration of such # Option 3 Unhandled exceptions, which don't cause a process termination, are considered like exceptions which cause the process to terminate and are marked `handled: false` -That would however make it impossible to differentiate between those exception types in the `session health` metric. +That would however make it impossible to differentiate between those exception types in the `session health` metric, filters, alerts, etc. # Approaches taken by other monitoring tools From ff8f808eafbe28441c73ed9b871b9674ceb5b18f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Sep 2022 14:24:30 +0200 Subject: [PATCH 17/21] link pr --- text/0010-exception-types.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 9fe77828..2432e955 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -18,6 +18,8 @@ This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsent Another issue is, that excpetions which don't cause the software to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). Currently, the session would be marked as `errored` instead of `crashed`. +The attribute `thread.errored` was added in the past for similar reasons, but it got [reverted](https://github.com/getsentry/relay/pull/306) for some reasons. + # Option 1 (recommended) Based on the problem stated above, I propose to introduce the types of `handled`, `unhandled`, `process termination` (this is the same as the current `handled`, but rephrased to avoid confusion). I'm open for better phrasing of those types, but I'll stick to those names for the rest of the RFC. The meaning of those types is as follows: From 3799712e3948ee0413b857462c9195731ae29d38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Thu, 29 Sep 2022 08:02:55 +0200 Subject: [PATCH 18/21] Update text/0010-exception-types.md Co-authored-by: Bruno Garcia --- text/0010-exception-types.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 2432e955..1f83cfc1 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -113,6 +113,7 @@ This list might be incomplete - Browser SDKs - Unity - React Native +- .NET (UnobservedTaskException) # Unresolved Questions From 3304a01b84a108baa46463e32ee392b6680e0255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Thu, 29 Sep 2022 08:03:10 +0200 Subject: [PATCH 19/21] Update text/0010-exception-types.md Co-authored-by: Bruno Garcia --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index 1f83cfc1..e06b5b4c 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -103,7 +103,7 @@ That would however make it impossible to differentiate between those exception t # Approaches taken by other monitoring tools -- Crashlytics just differentiates between manually caught exceptions and unhandled exceptions, regardless of wether they cause the process to terminate. +- Crashlytics just differentiates between manually caught exceptions and unhandled exceptions, regardless of whether they cause the process to terminate. # List of SDK to which this applies From 677b1d36346cadc0a2cf32a934f8cba6eca2ba5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Thu, 29 Sep 2022 08:03:24 +0200 Subject: [PATCH 20/21] Update text/0010-exception-types.md Co-authored-by: Bruno Garcia --- text/0010-exception-types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index e06b5b4c..d49d0ea7 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -18,7 +18,7 @@ This issue impacts for example the [Dart/Flutter SDK](https://github.com/getsent Another issue is, that excpetions which don't cause the software to exit but are unhandled, are not considered in the [`session health` metric](https://develop.sentry.dev/sdk/sessions/). Currently, the session would be marked as `errored` instead of `crashed`. -The attribute `thread.errored` was added in the past for similar reasons, but it got [reverted](https://github.com/getsentry/relay/pull/306) for some reasons. +The attribute `thread.errored` was added in the past for similar reasons, but it got [reverted on Relay](https://github.com/getsentry/relay/pull/306) and [Android](https://github.com/getsentry/sentry-android/pull/187). # Option 1 (recommended) From 3485305ded4f5c2bbb4b669de02451013f878f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Fri, 7 Oct 2022 14:07:42 +0200 Subject: [PATCH 21/21] Update text/0010-exception-types.md Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- text/0010-exception-types.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0010-exception-types.md b/text/0010-exception-types.md index d49d0ea7..521156cf 100644 --- a/text/0010-exception-types.md +++ b/text/0010-exception-types.md @@ -118,3 +118,4 @@ This list might be incomplete # Unresolved Questions - Are there any other exception types next to the ones metioned in this RFC? +- When there's an unhandled exception, we'd automatically finish the transaction bound to the scope, but only if the app crashes (process termination)? See [issue](https://github.com/getsentry/develop/issues/443).