Skip to content

QueryResource: Don't close JSON content on error.#17034

Merged
gianm merged 4 commits intoapache:masterfrom
gianm:fix-native-api
Sep 14, 2024
Merged

QueryResource: Don't close JSON content on error.#17034
gianm merged 4 commits intoapache:masterfrom
gianm:fix-native-api

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Sep 11, 2024

Following similar issues fixed in #11685 and #15880, this patch fixes a bug where QueryResource would write a closing array marker if it encountered an exception after starting to push results. This makes it difficult for callers to detect errors.

The prior patches didn't catch this problem because QueryResource uses the ObjectMapper in a unique way, through writeValuesAsArray, which doesn't respect the global AUTO_CLOSE_JSON_CONTENT setting.

Following similar issues fixed in apache#11685 and apache#15880, this patch fixes
a bug where QueryResource would write a closing array marker if it
encountered an exception after starting to push results. This makes it
difficult for callers to detect errors.

The prior patches didn't catch this problem because QueryResource uses
the ObjectMapper in a unique way, through writeValuesAsArray, which
doesn't respect the global AUTO_CLOSE_JSON_CONTENT setting.
@gianm gianm added the Bug label Sep 11, 2024
Copy link
Copy Markdown
Contributor

@abhishekagarwal87 abhishekagarwal87 left a comment

Choose a reason for hiding this comment

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

the IT failures are genuine.

@abhishekagarwal87 abhishekagarwal87 dismissed their stale review September 12, 2024 05:22

IT failures need to be looked into

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Sep 13, 2024

The issue was the new code wasn't using the fully-customized ObjectMapper from the toolchest, so it wasn't respecting parameters like resultAsArray. The new patch fixes that.

@gianm gianm merged commit d7be120 into apache:master Sep 14, 2024
@gianm gianm deleted the fix-native-api branch September 14, 2024 22:32
pranavbhole pushed a commit to pranavbhole/druid that referenced this pull request Sep 17, 2024
* QueryResource: Don't close JSON content on error.

Following similar issues fixed in apache#11685 and apache#15880, this patch fixes
a bug where QueryResource would write a closing array marker if it
encountered an exception after starting to push results. This makes it
difficult for callers to detect errors.

The prior patches didn't catch this problem because QueryResource uses
the ObjectMapper in a unique way, through writeValuesAsArray, which
doesn't respect the global AUTO_CLOSE_JSON_CONTENT setting.

* Fix usage of customized ObjectMappers.
@LakshSingla LakshSingla added this to the 31.0.0 milestone Oct 9, 2024
AmatyaAvadhanula pushed a commit to AmatyaAvadhanula/druid that referenced this pull request Oct 9, 2024
* QueryResource: Don't close JSON content on error.

Following similar issues fixed in apache#11685 and apache#15880, this patch fixes
a bug where QueryResource would write a closing array marker if it
encountered an exception after starting to push results. This makes it
difficult for callers to detect errors.

The prior patches didn't catch this problem because QueryResource uses
the ObjectMapper in a unique way, through writeValuesAsArray, which
doesn't respect the global AUTO_CLOSE_JSON_CONTENT setting.

* Fix usage of customized ObjectMappers.
abhishekagarwal87 pushed a commit that referenced this pull request Oct 9, 2024
* QueryResource: Don't close JSON content on error.

Following similar issues fixed in #11685 and #15880, this patch fixes
a bug where QueryResource would write a closing array marker if it
encountered an exception after starting to push results. This makes it
difficult for callers to detect errors.

The prior patches didn't catch this problem because QueryResource uses
the ObjectMapper in a unique way, through writeValuesAsArray, which
doesn't respect the global AUTO_CLOSE_JSON_CONTENT setting.

* Fix usage of customized ObjectMappers.

Co-authored-by: Gian Merlino <gianmerlino@gmail.com>
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.

3 participants