Skip to content

Conversation

djgraff209
Copy link
Contributor

@djgraff209 djgraff209 commented Aug 6, 2021

This fix changes the WebFluxRequestExecutingMessageHandler
The change specifically changes the contruction of the
WebClientResponseException to use the create factory method.

Fixes #3610.

This fix changes the WebFluxRequestExecutingMessageHandler
The change specifically changes the contruction of the
WebClientResponseException to use the `create` factory method.

This change addresses issue spring-projects#3610.
@pivotal-cla
Copy link

@djgraff209 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@djgraff209 Thank you for signing the Contributor License Agreement!

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, fix it as we discussed on Gitter: https://gitter.im/spring-projects/spring-integration?at=611069649484630efa2a4c46.

Also consider to cover it somehow with the test.
See #3612 for the place where you could inject the logic for your change.

Thank you!

@djgraff209
Copy link
Contributor Author

Will attempt to provide unit test to cover later today.

Updated code to use simplified exception construction that is more tolerant
  of larger payloads.
Updated unit tests to check for specific exception types.
Corrected checkstyle error in imports.
Corrected whitespace between casts.
Corrected trailing whitespace.
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Merging after adding your name to @author list.

Added @author entry.
@artembilan
Copy link
Member

Hm.
I meant that you are OK. That's already my responsibility to clean the fix for our standards, but since you have beaten me with that new commit I merge it without my hands 😄 .

The general rule if I request changes, you have to do them. Otherwise that means everything is up to me and you are good.

So, thank you for your patience with that nasty byte buffer issue and contribution.
Looking forward for more!

@djgraff209
Copy link
Contributor Author

Absolutely not a problem. I have been in the corporate world for many years and rigor like I'm trying to follow is meant mainly to reduce the burden on the owners.

Thanks again!

@artembilan artembilan closed this Aug 9, 2021
@artembilan artembilan reopened this Aug 9, 2021
@artembilan artembilan merged commit b949b90 into spring-projects:main Aug 9, 2021
@artembilan
Copy link
Member

... and cherry-picked to 5.4.x & 5.3.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebFluxRequestExecutingMessageHandler directly creates WebClientResponseException rather than using provided create(...) factory method

3 participants