|
| 1 | +# Netflix promises error handling survey |
| 2 | + |
| 3 | +This document presents the context and the findings of [a recent |
| 4 | +survey](https://docs.google.com/forms/d/1lM4TdTJcRbW2CDZw7Yj4Su75joGvK6DIkD95CenVRmk/viewanalytics) |
| 5 | +about promises error handling use cases that was done internally at Netflix. |
| 6 | + |
| 7 | +The goal of that survey was to get a better understanding on what promises error |
| 8 | +handling modes [recently proposed for the Node.js |
| 9 | +runtime](https://github.com/nodejs/node/pull/20097) are relevant for users, and |
| 10 | +how they would impact their ability to run applications reliably at Netflix |
| 11 | +while providing a good developer and operators experience. |
| 12 | + |
| 13 | +We hoped that it would help us inform the conversation in that PR, especially on |
| 14 | +aspects that did not reach broad consensus yet. |
| 15 | + |
| 16 | +The first section describes how this survey was conducted. Then, we provide a |
| 17 | +summary of the results that highlights what we think is relevant and |
| 18 | +interesting, but without any interpretation. |
| 19 | + |
| 20 | +The third section interprets those key items that we found interesting and |
| 21 | +presents what we think the key learnings can be. |
| 22 | + |
| 23 | +Finally, we conclude with the set of recommendations we want to make for the |
| 24 | +Node.js PR mentioned above and what other action items we think may be worth |
| 25 | +tackling in the future. |
| 26 | + |
| 27 | +## Disclaimer |
| 28 | + |
| 29 | +We're aware that this survey doesn't follow best practices in its methodology |
| 30 | +and implementation. We didn't expect this survey to provide us with a |
| 31 | +statistical confidence in its results. |
| 32 | + |
| 33 | +This is OK because the goal of this survey was to get a _general sense_ of what |
| 34 | +promises users inside of Netflix care about when it comes to error handling use |
| 35 | +cases, not an accurate picture of what all users inside and outside of Netflix |
| 36 | +actually rely on. |
| 37 | + |
| 38 | +## Methodology |
| 39 | + |
| 40 | +We used Google forms to conduct the survey. The total number of respondents is |
| 41 | +32 so far. The survey had been open for > 2 weeks at the time we collected and |
| 42 | +interpreted results. |
| 43 | + |
| 44 | +No question was mandatory, which explains why some questions have more answers |
| 45 | +than others. |
| 46 | + |
| 47 | +In addition to that, the question about asynchronously adding catch handlers to |
| 48 | +promises was added when the survey had already been filled out by half of the |
| 49 | +current number of respondents. We consider this to be acceptable, and we’d |
| 50 | +rather have some data about this question than no data. |
| 51 | + |
| 52 | +The survey didn't target any specific population of JavaScript users at Netflix. |
| 53 | +This was a conscious decision, as one of the goals was to get feedback from |
| 54 | +JavaScript users that had diverse technical backgrounds and that didn’t have the |
| 55 | +same biases as the users with which the team that conducted the survey interact |
| 56 | +on a day to day basis. |
| 57 | + |
| 58 | +In parallel of this survey, we did a small number of 30 minutes in-person |
| 59 | +interviews with four selected respondents to go deeper into some of the answers |
| 60 | +that were given when we thought that they could give us more insights. |
| 61 | + |
| 62 | +Those interviews were not recorded. We understand that this is a missed |
| 63 | +opportunity and that it lowers the credibility of this document, particularly |
| 64 | +when it comes to conclusions that have been significantly influenced by those |
| 65 | +interviews. We will strive to do better in the future. |
| 66 | + |
| 67 | +## Summary of survey results |
| 68 | + |
| 69 | +This section summarizes in prose the data that was collected by the survey and |
| 70 | +that we consider interesting. The next section (Interpretation of survey results |
| 71 | +and interviews) focuses on what we think that data means. |
| 72 | + |
| 73 | +For raw data, please consult the [survey |
| 74 | +results](https://docs.google.com/forms/d/1lM4TdTJcRbW2CDZw7Yj4Su75joGvK6DIkD95CenVRmk/viewanalytics). |
| 75 | + |
| 76 | +### Promises usage |
| 77 | + |
| 78 | +- Most of the respondents (90%) use promises. This could be because the survey |
| 79 | + was presented as a "promises survey", and thus users who didn't use promises |
| 80 | + may have self-selected out. There are at least three counter examples of this |
| 81 | + though, so this concern might not be valid. |
| 82 | + |
| 83 | +- Most respondents of promises seem to use native promises, but a non-negligible |
| 84 | + number of respondents use non-native promises. Among non-native promises |
| 85 | + implementations, Q and Bluebird seem to have the most users. |
| 86 | + |
| 87 | +- All respondents who are not using promises are using Rx observables instead. |
| 88 | + It's not clear whether the two serve identical use cases, but those users seem |
| 89 | + to at least have identified some overlap between the two and preferred Rx |
| 90 | + observables for several reasons: |
| 91 | + |
| 92 | + - Rx observables seem to be cancellable while promises are definitely not. |
| 93 | + - Historical context: promises were not necessarily available when those users |
| 94 | + had to pick a similar abstraction. |
| 95 | + - Issues with the error handling model of promises. |
| 96 | + |
| 97 | +### Async/Await usage |
| 98 | + |
| 99 | +- A significant number of respondents (~70%) seem to use async/await. |
| 100 | + |
| 101 | +- For others, reasons not to use async/await include: |
| 102 | + - Issues with the error handling model. |
| 103 | + - Runtime not supporting them (old version of JavaScript Core). |
| 104 | + |
| 105 | +### Promises error handling |
| 106 | + |
| 107 | +- A significant portion of respondents seem to differentiate between fatal and |
| 108 | + non-fatal errors when using promises. |
| 109 | + |
| 110 | +- A significant number of respondents seem to use different classes of errors to |
| 111 | + represent the difference between fatal and non-fatal errors. |
| 112 | + |
| 113 | +- A significant number of respondents consider that they should always attach a |
| 114 | + catch handler to a promise, however most of them don't seem to enforce it via |
| 115 | + tooling (e.g. static analysis tools). |
| 116 | + |
| 117 | +- Only one respondent mentioned that they add catch handlers asynchronously. |
| 118 | + When discussing their use case for it, they mentioned that they could achieve |
| 119 | + similar functionality differently (e.g. by emitting/listening on a 'error' |
| 120 | + event), and that the ability to add catch handlers _asynchronously_ was only a |
| 121 | + matter of user experience. |
| 122 | + |
| 123 | +- A significant number of respondents seem to consider that exiting the Node.js |
| 124 | + process on an unhandled promise rejection is a desirable behavior. |
| 125 | + |
| 126 | +- Most of the respondents that add an `'unhandledRejection'` event listener use |
| 127 | + it to log and exit the process. |
| 128 | + |
| 129 | +- However, most respondents seem not to use it, and a non-negligible part of |
| 130 | + them were not aware that the `'unhandledRejection'` event exists. |
| 131 | + |
| 132 | +- More respondents seem to be adding a listener for the `uncaughtException` |
| 133 | + event, and those handlers also almost always log messages and exit the |
| 134 | + process. However, a non-negligible number of respondents are not aware that |
| 135 | + this event exists. |
| 136 | + |
| 137 | +- No respondent seems to consider that exiting the Node.js process once a |
| 138 | + promise with an unhandled rejection is garbage collected is a desirable |
| 139 | + behavior. |
| 140 | + |
| 141 | +## Interpretation of survey results and interviews |
| 142 | + |
| 143 | +This section presents _our interpretation_ of the data that was collected and |
| 144 | +the four interviews we had with a selected sample of respondents. |
| 145 | + |
| 146 | +### Promises are used significantly at Netflix |
| 147 | + |
| 148 | +It is clear that a lot of JavaScript developers use promises at Netflix, |
| 149 | +including when writing Node.js applications. It also seems like a significant |
| 150 | +number of users are at least considering to use async/await. |
| 151 | + |
| 152 | +### Promises are confusing for users |
| 153 | + |
| 154 | +However, it's also clear that promises are confusing, even for developers who |
| 155 | +have a lot of experience with them. Every single one-on-one conversation we |
| 156 | +had with promises users highlighted at least one area of confusion with regards |
| 157 | +to how different JS runtimes implement promises and how they handle their |
| 158 | +errors. |
| 159 | + |
| 160 | +### Developers don't use static analysis tools to validate promises usage |
| 161 | + |
| 162 | +So far, we have seen that a significant number of users do not rely on static |
| 163 | +analysis tools to help them validate their usage of promises. As a result, it |
| 164 | +seems they _exclusively rely on runtime checks_ to perform this validation. |
| 165 | + |
| 166 | +### Exiting Node.js process on unhandled rejection seems to be considered as valid behavior |
| 167 | + |
| 168 | +A lot of respondents seem to be comfortable with Node.js exiting when a promise |
| 169 | +rejection is unhandled. The motivation for this behavior includes having better |
| 170 | +visibility on potentially hidden errors, and ensuring the process' state is |
| 171 | +consistent. |
| 172 | + |
| 173 | +The fact that the process exits synchronously or not does not seem to matter |
| 174 | +much for users. |
| 175 | + |
| 176 | +Some users are not comfortable with having the runtime dictate the behavior of |
| 177 | +the process when an unhandled rejection occurs, but they seem to be fine with |
| 178 | +being able to override the default behavior. |
| 179 | + |
| 180 | +### Exiting on garbage collection doesn't seem to be considered as a desirable behavior |
| 181 | + |
| 182 | +As part of the survey, no respondent chose this mode as a desirable behavior of |
| 183 | +the runtime. Moreover, without giving any hint, every interview highlighted that |
| 184 | +this was considered a potential source of confusion. We failed to find evidence |
| 185 | +of respondents who considered this was a desirable behavior of the runtime. |
| 186 | + |
| 187 | +### Attaching catch handlers asynchronously doesn't seem to be a required use case |
| 188 | + |
| 189 | +Only one respondent mentioned it as an important use case. However, after |
| 190 | +discussing this use case with them, they mentioned that the ability to |
| 191 | +asynchronously add catch handlers to promises wasn't a functional requirement, |
| 192 | +and that it was only a nice to have in order to provide better UX. |
| 193 | + |
| 194 | +## Conclusions |
| 195 | + |
| 196 | +In this section, we present what we think are relevant conclusions based on this |
| 197 | +survey that can inform future discussions in the [PR recently proposed for the |
| 198 | +Node.js runtime](https://github.com/nodejs/node/pull/20097) and more generally |
| 199 | +on the topic of promises debugging and error handling. |
| 200 | + |
| 201 | +### No evidence that exiting on GCed unhandled promise is desirable |
| 202 | + |
| 203 | +We haven't managed to find evidence of exiting on GC being a valid use case. As |
| 204 | +a result we are still leaning towards recommending not to add this behavior to |
| 205 | +the Node.js runtime. |
| 206 | + |
| 207 | +### Exiting on unhandled rejections could be well accepted by Node.js users |
| 208 | + |
| 209 | +We haven't found any evidence that, when providing the ability to override this |
| 210 | +behavior, having the runtime defaults to exit on unhandled rejections is |
| 211 | +undesirable. |
| 212 | + |
| 213 | +As a result, we will continue to lean towards recommending this as the default |
| 214 | +behavior for the Node.js runtime, as well as providing the ability to add a |
| 215 | +custom override. |
| 216 | + |
| 217 | +### Advocating for using static analysis tools could help |
| 218 | + |
| 219 | +It seems that using static code analysis tooling is not a common way for |
| 220 | +developers to validate the robustness of their code regarding promises error |
| 221 | +handling. |
| 222 | + |
| 223 | +If such static analysis tooling was broadly used, we could at least inform |
| 224 | +runtime behavior changes based on behavior that is enforced at build time. |
| 225 | + |
| 226 | +As a result, it seems worth it to explore advocating for using useful static |
| 227 | +code analysis tooling that encourages the best practices that we’d like the |
| 228 | +broader community to use and that are compatible with the debugging tools we’d |
| 229 | +like to use. |
| 230 | + |
| 231 | +### Focus on developers education may help reach consensus in the future |
| 232 | + |
| 233 | +All our conversations with promises users showed that there are a lot of |
| 234 | +different perspectives on why to use promises, how they work, and what best |
| 235 | +practices to use. |
| 236 | + |
| 237 | +Most of the time though, when digging deeper and when providing additional |
| 238 | +insights, it seemed that all interviewees reached a common ground on those |
| 239 | +topics. |
| 240 | + |
| 241 | +Our takeaway is that socializing challenges and best practices around using |
| 242 | +promises and their error handling model could go a long way towards building a |
| 243 | +shared understanding about promises and reaching a consensus on those topics. |
| 244 | + |
| 245 | +We think reaching a broader consensus will help steer the Node.js platform |
| 246 | +towards a more consistent behavior with regards to how promises are supported. |
| 247 | +We hope to be able to work on enabling those conversations in the near future. |
| 248 | + |
0 commit comments