Timeout implementation #163

RicardRC · 2025-03-16T13:38:36Z

No description provided.

First draft of generated satori cpp api Generate cpp grpcs for satori client Generalize git submodule urls wip satori grpc client and test Revert "Merge remote-tracking branch 'origin/private-build-rules' into satori-client-cpp" WIP build test Add dir tree to satori cmake generate cpp api First draft of generated satori cpp api Generate cpp grpcs for satori client Fix libhttpclient builds, rearrange build scripts for easier console builds Private build fixups protobuf + abseil PS5 wip All dependencies built wslay port added NDAed platforms build refactor Fix private builds factory methods Sync protobuf verion in public and private builds Remove unnecesasry compile definition libhttpclient build fixes Windows: align project and vcpkg deps on toolchain used. Fix wslay builds. Update to latest libHttpClient and provide build script for it. Also simplify bunch of build instructions switch to upstream libhttpclient Use aliases where possible for safety, cleanup libhttpclient build scripts GDK support for libhttpclient transport Use winhttp (via libhttpclient) on Windows Mac OSX Universal binary Update CHANGELOG.md Fix iOS builds Fix iphonesimulator builds Fix Linux builds Accurately list transports in README.md Generalize git submodule urls wip satori grpc client and test Revert "Merge remote-tracking branch 'origin/private-build-rules' into satori-client-cpp" WIP build test cmake minor fixes wip generate cpp api First draft of generated satori cpp api Generate cpp grpcs for satori client Generalize git submodule urls wip satori grpc client and test Revert "Merge remote-tracking branch 'origin/private-build-rules' into satori-client-cpp" WIP build test Add dir tree to satori cmake generate cpp api First draft of generated satori cpp api Generate cpp grpcs for satori client Fix libhttpclient builds, rearrange build scripts for easier console builds Private build fixups protobuf + abseil PS5 wip All dependencies built wslay port added NDAed platforms build refactor Fix private builds factory methods Sync protobuf verion in public and private builds Remove unnecesasry compile definition libhttpclient build fixes Windows: align project and vcpkg deps on toolchain used. Fix wslay builds. Update to latest libHttpClient and provide build script for it. Also simplify bunch of build instructions switch to upstream libhttpclient Use aliases where possible for safety, cleanup libhttpclient build scripts GDK support for libhttpclient transport Use winhttp (via libhttpclient) on Windows Mac OSX Universal binary Update CHANGELOG.md Fix iOS builds Fix iphonesimulator builds Fix Linux builds Accurately list transports in README.md Generalize git submodule urls wip satori grpc client and test Revert "Merge remote-tracking branch 'origin/private-build-rules' into satori-client-cpp" WIP build test cmake minor fixes wip wip setup cmake add

…rotobuf

Add additional info to Satori Call Get Flags. More defensive json parsing.

redbaron · 2025-03-17T09:29:46Z

I have changed base of this PR to more accurately reflect what is being changed

impl/httpCppRest/NHttpClientCppRest.cpp

…ded' into timeout-impl # Conflicts: # CMakeLists.txt

RicardRC · 2025-03-20T12:09:14Z

Client needs satori, so this branch now pulls in the full satori implementation too. I've not changed anything else from the original PR besides commit [555b2c4], but it's not substantial, only a rearrangement and what is necessary to get timeouts inside satori as well.

redbaron · 2025-03-20T13:39:49Z

impl/httpCppRest/CMakeLists.txt

cpprestsdk can probably go, AFAIK it is not used on any platform by default anymore

Yeah, it's not used anywhere. I will remove it on another PR, though, I don't want to cram this one with even more changes.

impl/httpCppRest/NHttpClientCppRest.cpp

Copilot

Pull Request Overview

This PR primarily focuses on the timeout implementation for HTTP clients and ensures that all endpoint URL parameters are properly encoded. Key changes include:

Introducing a timeout check in the RestClient creation within the ClientFactory.
Wrapping various URL parameters with encodeURIComponent in RestClient methods.
Adjusting default member initializations in BaseClient and updating the Java package in AndroidCA.

Reviewed Changes

Copilot reviewed 45 out of 61 changed files in this pull request and generated 1 comment.

File	Description
factory/ClientFactory.cpp	Adds a timeout implementation in createRestClient and reformats header includes.
core/core-rest/RestClient.cpp	Wraps URL parameters with encodeURIComponent for safe URL composition.
core/common/BaseClient.h	Sets a default value for _port and adjusts member ordering for consistency.
android/nativelib/src/main/java/com/heroiclabs/nakama/AndroidCA.java	Updates package naming and removes unused certificate imports.

Files not reviewed (16)

CMakeLists.txt: Language not supported
CMakePresets.json: Language not supported
android/app/build.gradle: Language not supported
android/build.gradle.kts: Language not supported
android/gradle.properties: Language not supported
android/gradle/wrapper/gradle-wrapper.properties: Language not supported
android/nativelib/build.gradle.kts: Language not supported
android/nativelib/src/main/AndroidManifest.xml: Language not supported
android/settings.gradle: Language not supported
android/settings.gradle.kts: Language not supported
cmake/triplets/arm64-v8a-android-heroic.cmake: Language not supported
cmake/triplets/armeabi-v7a-android-heroic.cmake: Language not supported
cmake/triplets/x86-64-android-heroic.cmake: Language not supported
cmake/triplets/x86-android-heroic.cmake: Language not supported
core/buildProtoFiles.cmake: Language not supported
factory/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (1)

factory/ClientFactory.cpp:63

Verify that using '>= 0' for the timeout check is intentional and that a timeout of 0 is meant to be applied to the HTTP transport; if a non-operational timeout is not desired, consider using '> 0' instead.

if (parameters.timeout >= std::chrono::milliseconds(0)) {

Copilot · 2025-05-27T22:35:54Z

core/common/BaseClient.h

@@ -476,12 +476,12 @@ namespace Nakama {
        ) override;

    protected:
-        int _port;
-        std::string _host;
+        int _port = -1;


[nitpick] Consider using a named constant (e.g., DEFAULT_PORT) instead of the magic number -1 to improve clarity and maintainability.

Suggested change

int _port = -1;

static constexpr int DEFAULT_PORT = -1;

int _port = DEFAULT_PORT;

Add ability to set timeouts to the rest api (milliseconds for the backends that support that much granularity, seconds for the ones that do not). --------- Co-authored-by: Maxim Ivanov <[email protected]> Co-authored-by: Maxim Ivanov <[email protected]>

redbaron and others added 13 commits October 10, 2024 13:43

Android WIP checkpoint

8ccc711

Ignore gradle generated files

fac5c7f

Fix nativelib android build

91447b7

Fix protobuf generation instructions

4711749

Temporarily append all the required includes in directory.

1d298cb

Remove unnecessary headers

e1b736e

WIP properly generate api from protobuf

1b42e13

WIP almost done with correctly generated api!

fcdaa92

Change to pre-included satori.proto file

c254f8d

WIP trying to get a protobuf_generate working

db99f97

Implement Satori Client CPP API hardcoded instead of generated from p…

1f9eef3

…rotobuf

Add Satori Client call to get flag overrides.

b0e2f66

Add additional info to Satori Call Get Flags. More defensive json parsing.

RicardRC requested a review from andresfelipemendez March 16, 2025 15:05

RicardRC marked this pull request as ready for review March 16, 2025 15:05

RicardRC requested a review from redbaron March 16, 2025 15:05

redbaron changed the base branch from master to private-build-rules March 17, 2025 09:29

redbaron reviewed Mar 17, 2025

View reviewed changes

impl/httpCppRest/NHttpClientCppRest.cpp Outdated Show resolved Hide resolved

redbaron reviewed Mar 17, 2025

View reviewed changes

impl/httpCppRest/NHttpClientCppRest.cpp Outdated Show resolved Hide resolved

redbaron and others added 3 commits March 18, 2025 17:00

Simple test run frop top level dir

5c3b1f2

Fix test build

f433106

Timeout implementation

d4f197d

RicardRC force-pushed the timeout-impl branch from 6ecafdb to d4f197d Compare March 19, 2025 09:59

andresfelipemendez approved these changes Mar 19, 2025

View reviewed changes

Merge remote-tracking branch 'remotes/origin/satori-client-cpp-hardco…

0b06cff

…ded' into timeout-impl # Conflicts: # CMakeLists.txt

RicardRC requested review from andresfelipemendez and redbaron March 20, 2025 12:09

redbaron reviewed Mar 20, 2025

View reviewed changes

andresfelipemendez reviewed Mar 21, 2025

View reviewed changes

impl/httpCppRest/NHttpClientCppRest.cpp Outdated Show resolved Hide resolved

Improve ergonomics

e9bde67

RicardRC force-pushed the timeout-impl branch from 555b2c4 to e9bde67 Compare March 21, 2025 18:40

Change from int to std::chrono::durations

dc478e2

RicardRC requested a review from Copilot May 27, 2025 22:35

Copilot AI reviewed May 27, 2025

View reviewed changes

RicardRC merged commit b132025 into private-build-rules May 27, 2025
1 of 2 checks passed

RicardRC deleted the timeout-impl branch May 27, 2025 23:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Timeout implementation #163

Timeout implementation #163

Uh oh!

RicardRC commented Mar 16, 2025

Uh oh!

redbaron commented Mar 17, 2025

Uh oh!

Uh oh!

Uh oh!

RicardRC commented Mar 20, 2025

Uh oh!

redbaron Mar 20, 2025

Uh oh!

RicardRC Mar 20, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 27, 2025

Uh oh!

Uh oh!

Uh oh!

	int _port = -1;
	static constexpr int DEFAULT_PORT = -1;
	int _port = DEFAULT_PORT;

Timeout implementation #163

Timeout implementation #163

Uh oh!

Conversation

RicardRC commented Mar 16, 2025

Uh oh!

redbaron commented Mar 17, 2025

Uh oh!

Uh oh!

Uh oh!

RicardRC commented Mar 20, 2025

Uh oh!

redbaron Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

RicardRC Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!