vert-x3 / vertx-rx

Reactive Extensions for Vert.x
Apache License 2.0
147 stars 73 forks source link

RxJava3: always propagate WS error to subscriber callback #308

Closed NilsRenaud closed 5 months ago

NilsRenaud commented 5 months ago

Fix rxJava 3 with the same fix as in 9170efdbdf33322a09a16af9c1e7dbb736832f0a (see #247)

RxJava3 has been forgotten when fixing this bug.

There is no need to call the RX onError handler since there is a potential Handler to call and the Subscriber is already done anyway

NilsRenaud commented 5 months ago

eclipsefdn/eca check failed because I signed off with the wrong username... It's fixed now.

tsegismont commented 5 months ago

Thanks for this pr @NilsRenaud !

The build failure looks related and the ECA check does not pass yet.

NilsRenaud commented 5 months ago

Yep, I'm on the build issue, and I've updated my commit to comply with eclispefdn check, but I don't know how to launch the check again. Could you relauch it ?

NilsRenaud commented 5 months ago

Oh no, am I too late to have this fix shipped in 4.5.8 ? ><'

tsegismont commented 5 months ago

Yes, this will go to 4.5.9

Seems the email sent to ECA checker is nils.renaud@forg*rock DOT com Is this correct?

NilsRenaud commented 5 months ago

It was at the time of the first commit, but I've updated it and signed off this new commit with a gmail address

NilsRenaud commented 5 months ago

up for the eca checker @tsegismont can't you relaunch it ?

tsegismont commented 5 months ago

@NilsRenaud I did, but it's not working. If you run git show -s --format='Commit: %h, Committer: %ce, Author: %ae' <SHA> you will see the two emails are different. You need to fix this.

tsegismont commented 5 months ago

Typically when setting up a project, I run:

git config user.email <address>
git config author.email <address>

And then you can try:

git commit --amend -s --author=<address>
git push --force
NilsRenaud commented 5 months ago

Done. Thanks @tsegismont I didn't know both fields could be differents ><'

tsegismont commented 5 months ago

@NilsRenaud can please you update the main branch as well?

NilsRenaud commented 5 months ago

OK, I'll create the PR soon

tsegismont commented 5 months ago

Thank you

NilsRenaud commented 5 months ago

PR ready: https://github.com/vert-x3/vertx-rx/pull/309