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.

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

Prevent Bluebird warning about a promise not being returned from a handler #91

Merged
merged 3 commits into from
Sep 17, 2021

Conversation

MattiasBuelens
Copy link
Owner

@MattiasBuelens MattiasBuelens commented Sep 17, 2021

The polyfill often creates new promises within a fulfillment/rejection handler of a different promise, without returning that new promise. Bluebird thinks this might be a mistake, and logs a warning when this happens. However, in our case, these are not mistakes.

Reproducing this issue is surprisingly easy. Even basic usage of the polyfill is enough to trigger these warnings:

<script src="https://cdn.jsdelivr.net/bluebird/latest/bluebird.js"></script>
<script src="https://unpkg.com/web-streams-polyfill@3.1.1/dist/polyfill.min.js"></script>
<script>
const rs = new ReadableStream({
  start(c) {
    c.enqueue('a');
    c.enqueue('b');
    c.close();
  }
});
const ws = new WritableStream({
  write(chunk, c) {
    console.log(chunk);
  }
});
rs.pipeTo(ws);
</script>

Fix it by returning null from all problematic fulfillment/rejection handlers, to suppress these warnings.

Supersedes #90.

@Setitch
Copy link

Setitch commented Sep 17, 2021

Oh my, You were faster than me - (sat at that hour ago again and started adding returns----, but im glad You found it^^ I also see you added optimization of setting only one function for the variables at time without overriding them

@MattiasBuelens
Copy link
Owner Author

MattiasBuelens commented Sep 17, 2021

Oh my, You were faster than me - (sat at that hour ago again and started adding returns)

Yeah. Luckily we were already using helper functions for adding fulfillment/rejection handlers to promises, so I could change the type from void to null there and then find and fix all type errors. (I absolutely love static types for refactorings like these! 😁)

Since it's not that critical of a bug (I hope), I'm not going to make a separate patch release for it. Instead, it'll go into the 4.0 release, if that's okay with you.

@Setitch
Copy link

Setitch commented Sep 17, 2021

For now i hide the warnings, it will take A WHILE for this changes to be incorporated into openpgp library and then to project using it, so i really dont mind. Im happy thought we could improve the codebase^_^

@MattiasBuelens MattiasBuelens merged commit 6eb297f into next Sep 17, 2021
@MattiasBuelens MattiasBuelens deleted the fix-bluebird-warnings branch September 17, 2021 14:18
@MattiasBuelens
Copy link
Owner Author

Released as v4.0.0-beta.2. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants