Skip to content

Conversation

@martin951
Copy link

Improve http_request to not hang awaiting callback

Improve http_request to not hang awaiting callback
@Structed
Copy link
Owner

Structed commented Oct 4, 2025

Thank you very much for your pull request!

However, I did ask you in the other issue to first discuss this. Can you please elaborate a bit more why you want to change like you proposed? Since this is a breaking change, I would like to understand your use-case.

Also, this needs documentation updates, and tests would be great as well!

Thank you!

@martin951
Copy link
Author

hey,

first of all everything is super new to me so yeah I am getting lost in my own solutions.

I am using Promises lib to wrap any functions that use post_dict.

It like allows me to simply await my functions in a convenient way.

I am trying to write the explanation as best as I can but no luck, so heres an example of my solution:

func UpdateNickname(playerid: String, new_nick: String) -> Promise:
	var body = {
		"PlayFabId": playerid,
		"DisplayName": new_nick
	}
	
	print("JSON TEST: ",JSON.stringify(new_nick))

	return Promise.new(func(resolve, reject):
		PlayFabManager.client.post_dict(
			body,
			"/Admin/UpdateUserTitleDisplayName",
			func(response: Dictionary) -> void:
				var result: Dictionary = {}
				print(response)

				if response.has("errorMessage"):
					# PlayFab returned an error (duplicate name, etc.)
					result["success"] = false
					result["error"] = response["errorMessage"]
					print("Nickname change failed: ", response["errorMessage"])
					resolve.call(result)
				elif response.has("data") and response["data"].has("DisplayName"):
					# Success
					result["success"] = true
					result["displayName"] = response["data"]["DisplayName"]
					print("Nickname successfully changed to: ", result["displayName"])
					resolve.call(result)
				else:
					# Unexpected structure
					result["success"] = false
					result["error"] = "Unexpected response"
					result["raw"] = response
					print("Unexpected nickname response: ", response)
					resolve.call(result)
				,
			_headers
		)
	)

@martin951
Copy link
Author

You handle similar functions in a controller class, lets say, and then you can conveniently await them anywhere in your code.

But if the callback never triggers, it hangs

@martin951
Copy link
Author

What is the intended flow of post_dict?

@Structed
Copy link
Owner

Structed commented Oct 4, 2025

Which Promises lib are you using?

I don't really like the unifying of 200 and 400 responses. When you look at the PlayFab REST API docs, all 400 HTTP responses always return an ApiErrorWrapper. So I really want to capture this. And for all other requests, there's a *Request model, and a respective *Response model (see this request). Thus, I want 200 responses to be always be deserialised to the matching Response model.

@martin951
Copy link
Author

Using this tiny utility, works great:
https://github.com/TheWalruzz/godot-promise/

So how would you go about this? About awaiting post_dict method?
In case of an error response, you'd want to keep track of which specific function caused it, if connected to the signal..

@Structed
Copy link
Owner

Structed commented Oct 4, 2025

So yea, I wanted a better way of getting to call the right callback, once the request has finished. Ideally, I want users to not set up the same conversion with .to_dict() every time they are using one of the pre-built APIs wrappers in godot-playfab, like Events.

So a promise system like the one you mentioned might be something to look at.

Another way of doing this, would be to uniquely track the requests, with an ID of sorts. The thing is, GDScript is very limited, and I also didn't really have time to look at all the new stuff that was introduced with the 4.x line of GDScript (as you can see, I don't even use await).

@Structed
Copy link
Owner

Structed commented Oct 4, 2025

so in essence: I would appreciate a solution that can actually correlate the request to the response and call a user-defined callback, prior to that, actually deserializing to the expected model.

If you can look into that, I'd be extremely thankful!

@martin951
Copy link
Author

That pretty much means having all available playfab functions defined in classes

@Structed
Copy link
Owner

Structed commented Oct 5, 2025

well, we won't have all functions and models defined, so we need to find away around that

@martin951
Copy link
Author

I'd like to thank you for your work, it helped my project substantially. In my specific case it is preferable to have the callback invoked no matter the response code. I believe it is preferable in general. I expect every http_request to return a response, instead of a redirect through signals in case of an error, it complicates things. My pull request does not neglect the error wrappers, its a good base - it simply passes the callback at all times, which I believe makes sense.

@Structed
Copy link
Owner

Structed commented Oct 7, 2025

I think you may be right here :)

I have a few other priorities, when those are done, I'll look at this again. Truth be told, this whole HTTP class needs to be redone anyways. It's not scalable and with 4.x, specifically 4.5, I have a lot more tools to make it better.

Thanks for your contribution! I think I'm going with this or at the very least, build on it!

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.

2 participants