Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable rule "promise/catch-or-return"? #1322

Open
voxpelli opened this issue Aug 22, 2018 · 17 comments · May be fixed by standard/eslint-config-standard#148
Open

Enable rule "promise/catch-or-return"? #1322

voxpelli opened this issue Aug 22, 2018 · 17 comments · May be fixed by standard/eslint-config-standard#148
Labels
accepted
Milestone
standard 17

Comments

@voxpelli
Copy link
Member

In 12.0.0 (#123) it could be useful to add the promise/catch-or-return to help people catch the cases where one might have missed a catch() or return?

Especially as unhandledRejection is deprecated and will start throwing.

Thoughts?
@LinusU
Copy link
Member

LinusU commented Aug 27, 2018

I think this sounds very good, we should run a test and see how many packages it would break 👍

@voxpelli
Copy link
Member Author

@LinusU: Agree, that would be nice. How does one run such a test? Someone who has a setup for that?

@LinusU
Copy link
Member

LinusU commented Aug 27, 2018

We should really write this down somewhere 😅

  1. Clone https://github.com/standard/standard
  2. edit eslintrc.json and add the rule
  3. npm it

@voxpelli
Copy link
Member Author

voxpelli commented Aug 27, 2018 •
edited

Result:

# tests 272
# pass  252
# fail  20

Most with a single or two failing lines.

Of the two that I looked closer at it seems to be that the final .then() has been a .then(func1, func2), which do enable an exception in func1 to be silently caught and thus is a correctly caught error.

Two examples:

@LinusU
Copy link
Member

LinusU commented Aug 27, 2018

Could you post the entire list of failing repos, then we can start sending PRs 😄

@voxpelli
Copy link
Member Author

Here's all the errors:

/Users/pelle/Sites/smallrepos/standard/tmp/fs-extra/lib/copy/copy.js:51:3: Expected catch() or return (promise/catch-or-return)

not ok 11 fs-extra (https://github.com/jprichardson/node-fs-extra)


/Users/pelle/Sites/smallrepos/standard/tmp/karma-browserstack-launcher/index.js:203:5: Expected catch() or return (promise/catch-or-return)

not ok 26 karma-browserstack-launcher (https://github.com/karma-runner/karma-browserstack-launcher)


/Users/pelle/Sites/smallrepos/standard/tmp/parse-torrent/bin/cmd.js:25:39: Expected catch() or return (promise/catch-or-return)

not ok 34 parse-torrent (https://github.com/webtorrent/parse-torrent)


/Users/pelle/Sites/smallrepos/standard/tmp/simple-peer/index.js:583:5: Expected catch() or return (promise/catch-or-return)

not ok 42 simple-peer (https://github.com/feross/simple-peer)


/Users/pelle/Sites/smallrepos/standard/tmp/write-file-atomic/index.js:44:3: Expected catch() or return (promise/catch-or-return)

not ok 44 write-file-atomic (https://github.com/iarna/write-file-atomic)


/Users/pelle/Sites/smallrepos/standard/tmp/napa/lib/pkg.js:112:7: Expected catch() or return (promise/catch-or-return)

not ok 53 napa (https://github.com/shama/napa)


/Users/pelle/Sites/smallrepos/standard/tmp/glob-promise/test/index.js:20:3: Expected catch() or return (promise/catch-or-return)

not ok 66 glob-promise (https://github.com/ahmadnassri/glob-promise)


/Users/pelle/Sites/smallrepos/standard/tmp/standard-engine/bin/cmd.js:95:5: Expected catch() or return (promise/catch-or-return)

not ok 73 standard-engine (https://github.com/flet/standard-engine)


/Users/pelle/Sites/smallrepos/standard/tmp/jsreport-express/lib/odata.js:62:7: Expected catch() or return (promise/catch-or-return)

not ok 74 jsreport-express (https://github.com/jsreport/jsreport-express)


/Users/pelle/Sites/smallrepos/standard/tmp/har-validator/test/promise.js:23:3: Expected catch() or return (promise/catch-or-return)

not ok 92 har-validator (https://github.com/ahmadnassri/har-validator)


/Users/pelle/Sites/smallrepos/standard/tmp/thunk-redis/test/commands/client.js:53:5: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/thunk-redis/test/commands/client.js:64:5: Expected catch() or return (promise/catch-or-return)

not ok 117 thunk-redis (https://github.com/thunks/thunk-redis)


/Users/pelle/Sites/smallrepos/standard/tmp/read-closest-package/index.js:28:5: Expected catch() or return (promise/catch-or-return)

not ok 139 read-closest-package (https://github.com/mattdesl/read-closest-package)


/Users/pelle/Sites/smallrepos/standard/tmp/fs-writefile-promise/test/index.js:17:3: Expected catch() or return (promise/catch-or-return)

not ok 151 fs-writefile-promise (https://github.com/ahmadnassri/fs-writefile-promise)


/Users/pelle/Sites/smallrepos/standard/tmp/ban-sensitive-files/bin/ban.js:40:1: Expected catch() or return (promise/catch-or-return)

not ok 162 ban-sensitive-files (https://github.com/bahmutov/ban-sensitive-files)


/Users/pelle/Sites/smallrepos/standard/tmp/socket.io-rpc-client/sample/node_client.js:6:1: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/socket.io-rpc-client/sample/node_client.js:8:5: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/socket.io-rpc-client/sample/node_client.js:11:5: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/socket.io-rpc-client/test/socket.io-rpc-client.spec.js:22:11: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/socket.io-rpc-client/test/socket.io-rpc-client.spec.js:103:7: Expected catch() or return (promise/catch-or-return)

not ok 200 socket.io-rpc-client (https://github.com/capaj/socket.io-rpc-client)


/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:16:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:19:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:26:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:29:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:36:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:39:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:47:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:52:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:56:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:65:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:70:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:75:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:79:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:87:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:92:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:96:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:105:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:110:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:115:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:119:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:126:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:131:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:136:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:140:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:149:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:154:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:160:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:164:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:172:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:175:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:180:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:183:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:188:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:191:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:196:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:199:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:203:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:211:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:214:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:219:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:222:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:227:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:230:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:235:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:238:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:243:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:246:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:251:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:254:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:259:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:262:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:267:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:270:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:274:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:282:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:287:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:292:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:295:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:300:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:305:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:310:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:313:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:317:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:324:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:327:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:330:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:333:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:336:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:339:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:342:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:347:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:350:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/addressbar/tests/index.js:354:3: Expected catch() or return (promise/catch-or-return)

not ok 208 addressbar (https://github.com/cerebral/addressbar)


/Users/pelle/Sites/smallrepos/standard/tmp/tape-promise/index.babel.js:33:27: Expected catch() or return (promise/catch-or-return)

not ok 255 tape-promise (https://github.com/jprichardson/tape-promise)


/Users/pelle/Sites/smallrepos/standard/tmp/instant.io/client/index.js:259:11: Expected catch() or return (promise/catch-or-return)

not ok 266 instant.io (https://github.com/webtorrent/instant.io)


/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/index.js:10:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-add-torrent.js:11:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-add-torrent.js:62:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-audio.js:9:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-torrent-list.js:13:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-torrent-list.js:32:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-torrent-list.js:62:3: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/webtorrent-desktop/test/test-video.js:9:3: Expected catch() or return (promise/catch-or-return)

not ok 268 webtorrent-desktop (https://github.com/webtorrent/webtorrent-desktop)


/Users/pelle/Sites/smallrepos/standard/tmp/studynotes/liveupdater/index.js:268:5: Expected catch() or return (promise/catch-or-return)

/Users/pelle/Sites/smallrepos/standard/tmp/studynotes/tools/transform-css.js:13:1: Expected catch() or return (promise/catch-or-return)

not ok 270 studynotes (https://github.com/feross/studynotes)

@LinusU
Copy link
Member

LinusU commented Aug 27, 2018 •
edited

Thank you! Cleaned it up a bit here:

  • fs-extra

    • fs-extra/lib/copy/copy.js:51:3: Expected catch() or return (promise/catch-or-return)

  • karma-browserstack-launcher

    • karma-browserstack-launcher/index.js:203:5: Expected catch() or return (promise/catch-or-return)

  • parse-torrent

    • parse-torrent/bin/cmd.js:25:39: Expected catch() or return (promise/catch-or-return)

  • simple-peer

    • simple-peer/index.js:583:5: Expected catch() or return (promise/catch-or-return)

  • write-file-atomic

    • write-file-atomic/index.js:44:3: Expected catch() or return (promise/catch-or-return)

  • napa

    • napa/lib/pkg.js:112:7: Expected catch() or return (promise/catch-or-return)

  • glob-promise

    • glob-promise/test/index.js:20:3: Expected catch() or return (promise/catch-or-return)

  • standard-engine

    • standard-engine/bin/cmd.js:95:5: Expected catch() or return (promise/catch-or-return)

  • jsreport-express

    • jsreport-express/lib/odata.js:62:7: Expected catch() or return (promise/catch-or-return)

  • har-validator

    • har-validator/test/promise.js:23:3: Expected catch() or return (promise/catch-or-return)

  • thunk-redis

    • thunk-redis/test/commands/client.js:53:5: Expected catch() or return (promise/catch-or-return)
    • thunk-redis/test/commands/client.js:64:5: Expected catch() or return (promise/catch-or-return)

  • read-closest-package

    • read-closest-package/index.js:28:5: Expected catch() or return (promise/catch-or-return)

  • fs-writefile-promise

    • fs-writefile-promise/test/index.js:17:3: Expected catch() or return (promise/catch-or-return)

  • ban-sensitive-files

    • ban-sensitive-files/bin/ban.js:40:1: Expected catch() or return (promise/catch-or-return)

  • socket.io-rpc-client

    • socket.io-rpc-client/sample/node_client.js:6:1: Expected catch() or return (promise/catch-or-return)
    • socket.io-rpc-client/sample/node_client.js:8:5: Expected catch() or return (promise/catch-or-return)
    • socket.io-rpc-client/sample/node_client.js:11:5: Expected catch() or return (promise/catch-or-return)
    • socket.io-rpc-client/test/socket.io-rpc-client.spec.js:22:11: Expected catch() or return (promise/catch-or-return)
    • socket.io-rpc-client/test/socket.io-rpc-client.spec.js:103:7: Expected catch() or return (promise/catch-or-return)

  • addressbar

    • addressbar/tests/index.js:16:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:19:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:26:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:29:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:36:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:39:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:47:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:52:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:56:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:65:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:70:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:75:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:79:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:87:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:92:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:96:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:105:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:110:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:115:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:119:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:126:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:131:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:136:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:140:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:149:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:154:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:160:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:164:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:172:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:175:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:180:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:183:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:188:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:191:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:196:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:199:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:203:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:211:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:214:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:219:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:222:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:227:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:230:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:235:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:238:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:243:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:246:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:251:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:254:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:259:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:262:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:267:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:270:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:274:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:282:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:287:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:292:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:295:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:300:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:305:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:310:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:313:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:317:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:324:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:327:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:330:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:333:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:336:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:339:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:342:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:347:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:350:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:354:3: Expected catch() or return (promise/catch-or-return)

  • tape-promise

    • tape-promise/index.babel.js:33:27: Expected catch() or return (promise/catch-or-return)

  • instant.io

    • instant.io/client/index.js:259:11: Expected catch() or return (promise/catch-or-return)

  • webtorrent-desktop

    • webtorrent-desktop/test/index.js:10:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-add-torrent.js:11:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-add-torrent.js:62:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-audio.js:9:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:13:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:32:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:62:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-video.js:9:3: Expected catch() or return (promise/catch-or-return)

  • studynotes

    • studynotes/liveupdater/index.js:268:5: Expected catch() or return (promise/catch-or-return)
    • studynotes/tools/transform-css.js:13:1: Expected catch() or return (promise/catch-or-return)

LinusU referenced this issue in LinusU/node-fs-extra Aug 27, 2018
@LinusU
Catch errors thrown by onInclude
6f2ac5f 
When using the `.then($1, $2)` function, any errors thrown from `$1` will *not* be delegated to `$2`. Changing to `.then($1).catch($2)` makes sure that even errors from `$1` gets delegated to `$2`.

Found by this proposed standard rule: standard/eslint-config-standard#129
LinusU referenced this issue in LinusU/karma-browserstack-launcher Aug 27, 2018
@LinusU
Catch errors thrown by client.createWorker
50556b6 
When using the `.then($1, $2)` function, any errors thrown from `$1` will *not* be delegated to `$2`. Changing to `.then($1).catch($2)` makes sure that even errors from `$1` gets delegated to `$2`.

Found by this proposed standard rule: standard/eslint-config-standard#129
LinusU referenced this issue in LinusU/parse-torrent Aug 27, 2018
@LinusU
Catch errors from stdin.buffer()
e4845e0 
Found by this proposed standard rule: standard/eslint-config-standard#129
@rstacruz
Copy link
Member

We should really write this down somewhere 😅

This seems like something that belongs in CONTRIBUTING.md :)

@voxpelli
Copy link
Member Author

voxpelli commented Jul 6, 2019

@feross @LinusU How do I/we push this forward?

@voxpelli
Copy link
Member Author

voxpelli commented Jul 6, 2019

Here's an up to date run

11 failing projects, 6.7% of total
  • simple-peer
    • simple-peer/index.js:693:5: Expected catch() or return (promise/catch-or-return)
  • standard-engine
    • standard-engine/bin/cmd.js:95:5: Expected catch() or return (promise/catch-or-return)
  • har-validator
    • har-validator/test/promise.js:23:3: Expected catch() or return (promise/catch-or-return)
  • read-closest-package
    • read-closest-package/index.js:28:5: Expected catch() or return (promise/catch-or-return)
  • fs-writefile-promise
    • fs-writefile-promise/test/index.js:17:3: Expected catch() or return (promise/catch-or-return)
  • ban-sensitive-files
    • ban-sensitive-files/bin/ban.js:40:1: Expected catch() or return (promise/catch-or-return)
  • addressbar
    • addressbar/tests/index.js:16:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:19:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:26:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:29:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:36:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:39:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:47:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:52:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:56:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:65:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:70:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:75:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:79:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:87:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:92:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:96:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:105:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:110:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:115:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:119:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:126:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:131:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:136:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:140:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:149:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:154:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:160:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:164:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:172:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:175:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:180:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:183:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:188:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:191:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:196:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:199:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:203:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:211:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:214:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:219:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:222:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:227:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:230:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:235:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:238:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:243:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:246:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:251:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:254:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:259:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:262:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:267:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:270:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:274:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:282:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:287:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:292:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:295:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:300:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:305:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:310:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:313:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:317:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:324:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:327:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:330:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:333:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:336:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:339:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:342:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:347:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:350:3: Expected catch() or return (promise/catch-or-return)
    • addressbar/tests/index.js:354:3: Expected catch() or return (promise/catch-or-return)
  • tape-promise
    • tape-promise/index.babel.js:68:36: Expected catch() or return (promise/catch-or-return)
    • tape-promise/tests/async.test.js:15:3: Expected catch() or return (promise/catch-or-return)
    • tape-promise/tests/async.test.js:18:7: Expected catch() or return (promise/catch-or-return)
  • instant.io
    • instant.io/client/index.js:259:11: Expected catch() or return (promise/catch-or-return)
  • studynotes
    • studynotes/liveupdater/index.js:268:5: Expected catch() or return (promise/catch-or-return)
    • studynotes/tools/transform-css.js:13:1: Expected catch() or return (promise/catch-or-return)
  • webtorrent-desktop
    • webtorrent-desktop/test/index.js:10:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-add-torrent.js:11:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-add-torrent.js:62:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-audio.js:9:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:13:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:32:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-torrent-list.js:62:3: Expected catch() or return (promise/catch-or-return)
    • webtorrent-desktop/test/test-video.js:9:3: Expected catch() or return (promise/catch-or-return)

voxpelli referenced this issue in voxpelli/eslint-config-standard Jul 6, 2019
@voxpelli
Add catch-or-return
d4ddf83 
Fixes #129
@feross
Copy link
Member

feross commented Jul 10, 2019 •
edited

@voxpelli Can you make a comment here (#1321) to propose this for inclusion in standard 14? Thanks!

@feross
Copy link
Member

feross commented Jul 10, 2019

@voxpelli That test run doesn't look too bad. If you have time, one thing that would help is sending PRs to the failing repos to fix the issues. That makes it less work for me to add the rule when the time comes (less code for me to change while working on the release; and fewer repos to mark as "disabled" in order to keep the tests passing)

@feross
Copy link
Member

feross commented Jul 11, 2019

I'm definitely down to try enabling this with { allowThen: true, allowFinally: true } since it still seems super useful with those options to catch cases where no attempt at error handling was made.

Regarding the allowThen option, can someone summarize the problem with using .then(success, fail) instead of .then(success).catch(fail)?

@feross feross transferred this issue from standard/eslint-config-standard Jul 11, 2019
@feross feross added this to the standard v14 milestone Jul 11, 2019
@LinusU
Copy link
Member

LinusU commented Jul 11, 2019

Regarding the allowThen option, can someone summarize the problem with using .then(success, fail) instead of .then(success).catch(fail)?

If the functions success throws, the thrown error will be eaten and result in the promise returned from then being rejected. If you don't do anything with that Promise (e.g. not returning it or .catching it) that error will go unreported ☺️

@voxpelli voxpelli linked a pull request Aug 2, 2019 that will close this issue
Open
@feross feross modified the milestones: standard 14, standard 15 Aug 15, 2019
@feross feross modified the milestones: standard 15, standard 16 Oct 22, 2020
@feross feross modified the milestones: standard 16, standard 17 Oct 29, 2020
@voxpelli
Copy link
Member Author

Starting with Node 15 which was just released, unhandled rejections will start throwing, so probably even more reason to add this.

I'm at your disposal to get this added @feross 👍

@feross
Copy link
Member

feross commented Oct 29, 2020

@voxpelli Can you propose a configuration for the rule? As well as testing ecosystem breakage with that setting?

// clone repo
npm install
// edit the eslint.json file, adding a "rules" section with your proposed rule
npm test

Report the final test output, number of tests passed/failed. You can also share representative examples of failures that you agree with and ones that are not ideal.

Thanks for offering to help!

@feross
Copy link
Member

feross commented Nov 10, 2020

@voxpelli I just noticed your comment here: standard/eslint-config-standard#148 (comment) Is your latest opinion that this shouldn't be merged as-is because of the false positives?

@voxpelli voxpelli mentioned this issue Sep 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted
Projects
The Standard Project
Status: No status
Milestone
standard 17
Development

Successfully merging a pull request may close this issue.

Add catch-or-return voxpelli/eslint-config-standard
4 participants
@voxpelli @rstacruz @feross @LinusU