Skip to content

Conversation

@voxpelli
Copy link
Member

@voxpelli voxpelli commented Jul 6, 2019

@LinusU
Copy link
Member

LinusU commented May 28, 2020

Ecosystem breakage report:

  • fs-extra
    • lib/copy/copy.js:54:3
  • simple-peer
    • index.js:701:7
  • standard-engine
    • bin/cmd.js:95:5
  • webtorrent
    • lib/file.js:109:5
    • lib/file.js:118:5
  • har-validator
    • test/promise.js:23:3
  • read-closest-package
    • index.js:28:5
  • fs-writefile-promise
    • test/index.js:17:3
  • aerospike
    • test/batch_exists.js:69:5
    • test/batch_get.js:69:5
    • test/batch_select.js:69:5
    • test/command_queue.js:29:7
    • test/command_queue.js:33:11
    • test/command_queue.js:46:7
    • test/command_queue.js:50:11
    • test/command_queue.js:63:7
    • test/get.js:116:5
  • ban-sensitive-files
    • bin/ban.js:40:1
  • addressbar
    • tests/index.js:16:3
    • tests/index.js:19:3
    • tests/index.js:26:3
    • tests/index.js:29:3
    • tests/index.js:36:3
    • tests/index.js:39:3
    • tests/index.js:47:3
    • tests/index.js:52:3
    • tests/index.js:56:3
    • tests/index.js:65:3
    • tests/index.js:70:3
    • tests/index.js:75:3
    • tests/index.js:79:3
    • tests/index.js:87:3
    • tests/index.js:92:3
    • tests/index.js:96:3
    • tests/index.js:105:3
    • tests/index.js:110:3
    • tests/index.js:115:3
    • tests/index.js:119:3
    • tests/index.js:126:3
    • tests/index.js:131:3
    • tests/index.js:136:3
    • tests/index.js:140:3
    • tests/index.js:149:3
    • tests/index.js:154:3
    • tests/index.js:160:3
    • tests/index.js:164:3
    • tests/index.js:172:3
    • tests/index.js:175:3
    • tests/index.js:180:3
    • tests/index.js:183:3
    • tests/index.js:188:3
    • tests/index.js:191:3
    • tests/index.js:196:3
    • tests/index.js:199:3
    • tests/index.js:203:3
    • tests/index.js:211:3
    • tests/index.js:214:3
    • tests/index.js:219:3
    • tests/index.js:222:3
    • tests/index.js:227:3
    • tests/index.js:230:3
    • tests/index.js:235:3
    • tests/index.js:238:3
    • tests/index.js:243:3
    • tests/index.js:246:3
    • tests/index.js:251:3
    • tests/index.js:254:3
    • tests/index.js:259:3
    • tests/index.js:262:3
    • tests/index.js:267:3
    • tests/index.js:270:3
    • tests/index.js:274:3
    • tests/index.js:282:3
    • tests/index.js:287:3
    • tests/index.js:292:3
    • tests/index.js:295:3
    • tests/index.js:300:3
    • tests/index.js:305:3
    • tests/index.js:310:3
    • tests/index.js:313:3
    • tests/index.js:317:3
    • tests/index.js:324:3
    • tests/index.js:327:3
    • tests/index.js:330:3
    • tests/index.js:333:3
    • tests/index.js:336:3
    • tests/index.js:339:3
    • tests/index.js:342:3
    • tests/index.js:347:3
    • tests/index.js:350:3
    • tests/index.js:354:3
  • tape-promise
    • index.babel.js:68:36
    • tests/async.test.js:15:3
    • tests/async.test.js:18:7
  • instant.io
    • client/index.js:267:11
  • studynotes.org
    • liveupdater/index.js:261:5
    • tools/transform-css.js:13:1
  • bitmidi.com
    • src/api/midi.js:143:5
  • webtorrent-desktop
    • bin/package.js:530:5
    • bin/package.js:564:5
    • src/renderer/webtorrent.js:360:3
    • test/index.js:10:3
    • test/test-add-torrent.js:11:3
    • test/test-add-torrent.js:60:3
    • test/test-audio.js:9:3
    • test/test-torrent-list.js:13:3
    • test/test-torrent-list.js:32:3
    • test/test-torrent-list.js:62:3
    • test/test-video.js:9:3
  • sinon-as-promised
    • examples/angular/test.js:22:5
    • test.js:26:5
    • test.js:28:7
    • test.js:37:5
  • testdouble
    • test/safe/callback.test.js:128:9
    • test/safe/when.test.js:163:9
    • test/safe/when.test.js:171:9
    • test/safe/when.test.js:172:11
    • test/safe/when.test.js:182:9
    • test/safe/when.test.js:189:9
    • test/safe/when.test.js:190:11
    • test/safe/when.test.js:220:9
    • test/safe/when.test.js:227:9
  • fastify
    • lib/contentTypeParser.js:116:7
    • lib/contentTypeParser.js:200:7
    • lib/hooks.js:138:9
    • lib/hooks.js:158:7
    • lib/hooks.js:198:7
    • lib/route.js:411:7
    • lib/wrapThenable.js:16:3
    • test/content-length.test.js:154:3
    • test/decorator.test.js:707:3
    • test/inject.test.js:347:3
    • test/internals/reply.test.js:1360:5
    • test/internals/reply.test.js:1374:5
    • test/listen.test.js:98:3
    • test/listen.test.js:113:3
    • test/listen.test.js:210:3
    • test/listen.test.js:220:3
    • test/listen.test.js:230:3
    • test/listen.test.js:240:3
    • test/listen.test.js:250:3
    • test/listen.test.js:260:3
    • test/listen.test.js:349:3

I don't have much time now but from a quick glance on some of them it seems like they are potentially false positives, which is not great 🤔

I think we need to do some more digging here...

@voxpelli
Copy link
Member Author

voxpelli commented Jun 2, 2020

Right, yeah for sinon-as-promised (which has been superseded by sinon 2.x) it's a bluebird + tape thing set up to handle the unhandled rejections:

https://github.com/bendrucker/sinon-as-promised/blob/c0fa8e411c450c4399be911a2da6f9350e5e9ef4/test.js#L9-L13

For fastify there seems to be a combination of using a .then(handleResolution, catchRejection) and specifics in test setup.

Maybe we would need to get an option to treat such calls as ok @LinusU?

@voxpelli
Copy link
Member Author

I would be 👍 on shipping this as a warn in 17.0.0, thoughts @feross @LinusU @divlo and others?

@LinusU
Copy link
Member

LinusU commented Oct 15, 2021

I'm not too sure about this, from the discussion in jprichardson/node-fs-extra#622 it seems like there isn't really a straight forward way to work around this.

I think that we should have some suggestions for the affected packages on what they should change to? 🤔

@voxpelli
Copy link
Member Author

@LinusU Good feedback 👍 I'm leaning more towards this after having slept on it as well. Lets try and target this for 18.0.0 instead then (I've set up some milestones to draft it up)

@rostislav-simonik rostislav-simonik added the scope:rule Concerning linting rules label Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:rule Concerning linting rules

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

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

4 participants