Skip to content

CXXCBC-580: Report last error when timing out on columnar query retries#650

Merged
DemetrisChr merged 2 commits intocouchbase:mainfrom
DemetrisChr:CXXCBC-580-last-err
Aug 30, 2024
Merged

CXXCBC-580: Report last error when timing out on columnar query retries#650
DemetrisChr merged 2 commits intocouchbase:mainfrom
DemetrisChr:CXXCBC-580-last-err

Conversation

@DemetrisChr
Copy link
Copy Markdown
Contributor

@DemetrisChr DemetrisChr commented Aug 28, 2024 •
edited
Loading

Motivation

The Error/Retry handling RFC Revision 8 requires that we list the last error code & message that was retried when reporting a timeout

Changes

  • Include a last_errors field in the error context that lists the set of server errors that were retried, when the error is a timeout resulting from retries.
  • FIX: Timeout should be initialized before the HTTP request in pending_query_operation

Results

Tested with FIT (via Python) - test passed

@avsej
Copy link
Copy Markdown
Member

avsej commented Aug 29, 2024

Interesting, why it started complaining about missing <string> header in core\platform\string_hex.h?

    24>D:\a\couchbase-cxx-client\couchbase-cxx-client\core\platform\string_hex.cc(95,18): error C2039: 'to_hex': is not a member of 'couchbase::core' [D:\a\couchbase-cxx-client\couchbase-cxx-client\cmake-build-tests\core\platform\couchbase_platform.vcxproj]
             D:\a\couchbase-cxx-client\couchbase-cxx-client\core\platform\string_hex.h(22,11):
             see declaration of 'couchbase::core'

@DemetrisChr, could you check it? and open separate PR to fix it if necessary.

It is indeed using std::string without #include <string> and so far it looks like whenever the header was included the <string> header was already there.

couchbase-cxx-client/core/platform/string_hex.h

Lines 35 to 36 in 9c445ae

std::string
to_hex(std::uint8_t val);

@DemetrisChr
Copy link
Copy Markdown
Contributor Author

DemetrisChr commented Aug 30, 2024 •
edited
Loading

@avsej Opened #652

thejcfactor
thejcfactor approved these changes Aug 30, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@thejcfactor thejcfactor left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@DemetrisChr DemetrisChr merged commit c3cb008 into couchbase:main Aug 30, 2024
@DemetrisChr DemetrisChr deleted the CXXCBC-580-last-err branch August 30, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thejcfactor thejcfactor thejcfactor approved these changes

@avsej avsej Awaiting requested review from avsej

@Matt-Woz Matt-Woz Awaiting requested review from Matt-Woz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DemetrisChr @avsej @thejcfactor