[eslint] prevent async Promise constructor mistakes by spalger · Pull Request #110349 · elastic/kibana

spalger · 2021-08-26T21:49:38Z

After fixing #109560 I did a little searching and found many cases where we are passing async functions to the Promise constructor without handling the potential of a rejected promise being created by said async function. This PR implements a custom ESLint rule which finds such situations and fixes them by wrapping the body of the function in a try/catch and routing errors caught to reject().

I don't think this produces great code, but it's an improvement over having unhandled promise rejections.

In #109565 I had manually fixed each violation and detailed some strategies for fixing this type of problem for those who are curious, but it felt like I was prescribing a code style in places and felt like we should really prevent this happening in the future with an ESLint rule. Thankfully, my work in #110161 helped me feel more confident in writing fixes like this.

…hidden-promise-constructor-rejections

elasticmachine · 2021-08-31T05:06:41Z

Pinging @elastic/kibana-operations (Team:Operations)

matschaffer

Approving on behalf of @elastic/stack-monitoring-ui - the changes look good and might even fix some of the error handling issues we've had come up with the logstash pipeline view via SDH.

YulNaumenko

Alerting changes LGTM!

ppisljar

app services changes lgtm

azasypkin

LGTM, just one note.

x-pack/plugins/security/server/session_management/session_index.ts

sabarasaba

Stack management changes LGTM!

thomasneirynck

maps/* and maps_ems/* changes ok. thx!

…g, fixing manually

…hidden-promise-constructor-rejections

FrankHassanabad

LGTM,

I searched for security_solution and saw the two areas that were fixed.

dej611

Code review LGTM for @elastic/kibana-vis-editors files

spalger · 2021-08-31T19:30:15Z

@elasticmachine merge upstream

…jections

kibanamachine · 2021-08-31T21:54:10Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`indexManagement`	1.3MB	1.3MB	+80.0B
`ml`	6.0MB	6.0MB	+300.0B
`visTypeTable`	104.0KB	104.0KB	+32.0B
`visTypeVislib`	564.8KB	564.9KB	+40.0B
total			+452.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`dataVisualizer`	16.2KB	16.2KB	+40.0B
`fileUpload`	23.6KB	23.7KB	+40.0B
`maps`	78.1KB	78.2KB	+40.0B
`mapsEms`	10.9KB	10.9KB	+40.0B
total			+160.0B

History

💔 Build #149855 failed 1d3bedf
💚 Build #149644 succeeded 3b2fb32
💔 Build #149127 failed d00dd02

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Co-authored-by: spalger <spalger@users.noreply.github.com>

kibanamachine · 2021-08-31T21:57:20Z

💔 Backport failed

Status	Branch	Result
❌	7.14	Commit could not be cherrypicked due to conflicts
❌	7.15	Commit could not be cherrypicked due to conflicts
✅	7.x

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 110349

Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts

Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # src/plugins/charts/public/services/active_cursor/use_active_cursor.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts # x-pack/plugins/task_manager/server/task_scheduling.ts

…110728) * [eslint] prevent async Promise constructor mistakes (#110349) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts * autofix additional violation Co-authored-by: spalger <spalger@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…110731) * [eslint] prevent async Promise constructor mistakes (#110349) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # src/plugins/charts/public/services/active_cursor/use_active_cursor.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts # x-pack/plugins/task_manager/server/task_scheduling.ts * update yarn.lock file * add @babel/generator dependency * autofix additional violation * update yarn.lock Co-authored-by: spalger <spalger@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…110727) * [eslint] prevent async Promise constructor mistakes (#110349) Co-authored-by: spalger <spalger@users.noreply.github.com> * autofix one more violation Co-authored-by: Spencer <email@spalger.com> Co-authored-by: spalger <spalger@users.noreply.github.com>

spalger added 2 commits August 26, 2021 14:44

[eslint] prevent async Promise constructor mistakes

75f6188

autofix all violation

d00dd02

spalger added Team:Operations Kibana-Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v7.15.0 v7.16.0 labels Aug 26, 2021

spalger added 2 commits August 30, 2021 17:05

Merge branch 'master' of github.com:elastic/kibana into implement/no-…

2dcf72c

…hidden-promise-constructor-rejections

autofix more violations

3b2fb32

spalger marked this pull request as ready for review August 31, 2021 05:06

spalger requested a review from a team August 31, 2021 05:06

spalger requested a review from a team as a code owner August 31, 2021 05:06

spalger requested a review from a team August 31, 2021 05:06

spalger requested review from a team as code owners August 31, 2021 05:06

spalger requested a review from a team August 31, 2021 05:06

spalger requested review from a team as code owners August 31, 2021 05:06

spalger requested a review from a team August 31, 2021 05:06

matschaffer approved these changes Aug 31, 2021

View reviewed changes

tylersmalley approved these changes Aug 31, 2021

View reviewed changes

YulNaumenko approved these changes Aug 31, 2021

View reviewed changes

ppisljar approved these changes Aug 31, 2021

View reviewed changes

azasypkin approved these changes Aug 31, 2021

View reviewed changes

x-pack/plugins/security/server/session_management/session_index.ts Show resolved Hide resolved

mshustov approved these changes Aug 31, 2021

View reviewed changes

sabarasaba approved these changes Aug 31, 2021

View reviewed changes

darnautov approved these changes Aug 31, 2021

View reviewed changes

thomasneirynck approved these changes Aug 31, 2021

View reviewed changes

JasonStoltz approved these changes Aug 31, 2021

View reviewed changes

timroes removed the request for review from a team August 31, 2021 14:57

spalger added 2 commits August 31, 2021 08:46

Reprinting the massive promise contructor caused some weird formattin…

2650f54

…g, fixing manually

Merge branch 'master' of github.com:elastic/kibana into implement/no-…

1d3bedf

…hidden-promise-constructor-rejections

spalger requested a review from a team as a code owner August 31, 2021 15:59

FrankHassanabad approved these changes Aug 31, 2021

View reviewed changes

spalger enabled auto-merge (squash) August 31, 2021 16:21

spalger added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 31, 2021

dej611 approved these changes Aug 31, 2021

View reviewed changes

Merge branch 'master' into implement/no-hidden-promise-constructor-re…

6ea1729

…jections

spalger merged commit 72f6700 into elastic:master Aug 31, 2021

spalger deleted the implement/no-hidden-promise-constructor-rejections branch August 31, 2021 21:55

kibanamachine mentioned this pull request Aug 31, 2021

[7.x] [eslint] prevent async Promise constructor mistakes (#110349) #110727

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 31, 2021

[eslint] prevent async Promise constructor mistakes (elastic#110349)

81be66a

Co-authored-by: spalger <spalger@users.noreply.github.com>

spalger mentioned this pull request Aug 31, 2021

[7.15] [eslint] prevent async Promise constructor mistakes (#110349) #110728

Merged

spalger mentioned this pull request Aug 31, 2021

[7.14] [eslint] prevent async Promise constructor mistakes (#110349) #110731

Merged

spalger mentioned this pull request Sep 8, 2021

[eslint] add rule to forbid async forEach bodies #111637

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[eslint] prevent async Promise constructor mistakes#110349

[eslint] prevent async Promise constructor mistakes#110349
spalger merged 7 commits intoelastic:masterfrom
spalger:implement/no-hidden-promise-constructor-rejections

spalger commented Aug 26, 2021

Uh oh!

elasticmachine commented Aug 31, 2021

Uh oh!

matschaffer left a comment

Uh oh!

YulNaumenko left a comment

Uh oh!

ppisljar left a comment

Uh oh!

azasypkin left a comment

Uh oh!

Uh oh!

sabarasaba left a comment

Uh oh!

thomasneirynck left a comment

Uh oh!

FrankHassanabad left a comment

Uh oh!

dej611 left a comment

Uh oh!

spalger commented Aug 31, 2021

Uh oh!

kibanamachine commented Aug 31, 2021

Uh oh!

kibanamachine commented Aug 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

Conversation

spalger commented Aug 26, 2021

Uh oh!

elasticmachine commented Aug 31, 2021

Uh oh!

matschaffer left a comment

Choose a reason for hiding this comment

Uh oh!

YulNaumenko left a comment

Choose a reason for hiding this comment

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sabarasaba left a comment

Choose a reason for hiding this comment

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

FrankHassanabad left a comment

Choose a reason for hiding this comment

Uh oh!

dej611 left a comment

Choose a reason for hiding this comment

Uh oh!

spalger commented Aug 31, 2021

Uh oh!

kibanamachine commented Aug 31, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

kibanamachine commented Aug 31, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants