GodotFirebase icon indicating copy to clipboard operation
GodotFirebase copied to clipboard

query with no results breaks system [BUG]

Open paperjack93 opened this issue 4 years ago • 8 comments

Describe the bug Make a query that is sure to have no results (ex non-existing collection as from) Errors appear: [Firebase Error] >> Offline queries are currently unsupported and another one

Expected behavior return empty array

paperjack93 avatar Oct 23 '21 16:10 paperjack93

It sends empty body and 404 response_code. The empty body makes the system think its offline response, and it breaks

paperjack93 avatar Oct 23 '21 16:10 paperjack93

@paperjack93 I tried this issue a few different ways.

If I query a collection that does not exist I get an error that reflects that image

If I run a query on a collection that should return nothing (In my example I query a collection for 'points', but no document has that) It seems to be working as intended and I get no error. image

I just get an array that is size 0.

BearDooks avatar Oct 28 '21 17:10 BearDooks

@paperjack93 I have tried everything I can think of and I can't replicate the issue. Have you had any luck?

Thanks

Chuck

BearDooks avatar Nov 02 '21 00:11 BearDooks

This is definitely a thing. I don't have much insight on how to reproduce this BUT I have only observed this on Android. It happens rather infrequently but when it does it makes everything explode. @paperjack93 was correct in that because the body is empty it thinks it's an offline query.

firestore_task.gd

var offline: bool = typeof(bod) == TYPE_NIL

And because bod isn't a dictionary it doesn't think it failed:

var failed: bool = bod is Dictionary and bod.has("error") and response_code != HTTPClient.RESPONSE_OK
if not cache_path.empty() and not failed and Firebase.Firestore.persistence_enabled:
    ...

and we eventually hit:

if not bod.empty() and offline:
    ...

and we crash because we're trying to call .empty() on null. Even if you let it get past this point it's still going to fail:

match action:
    Task.TASK_LIST:
	error = bod[0].error
	emit_signal("task_list_error", error.code, error.status, error.message)
    Task.TASK_QUERY:
	error = bod[0].error
	emit_signal("task_query_error", error.code, error.status, error.message)
    _:
	error = bod.error
	emit_signal("task_error", error.code, error.status, error.message)

Trying to index bod here is going to cause a crash. I was able to get around this by creating a new signal called bad_response(code) and emitting it inside of _on_request_completed when we detect HTTPClient.RESPONSE_NOT_FOUND. Inside of firestore.gd I connect to the signal when the task is created and emit it back out accordingly. This way I can at least show the user an error message when this happens.

## Emitted when a request is [b]not[/b] successfully completed.
## @arg-types Dictionary
signal bad_response(code)
signal task_error(code, status, message)
signal task_query_error(code, status, message)
signal task_list_error(code, status, message)

...

func _on_request_completed(result : int, response_code : int, headers : PoolStringArray, body : PoolByteArray) -> void:
    if response_code == HTTPClient.RESPONSE_NOT_FOUND:
        emit_signal("bad_response", response_code)
	return

...

This isn't much of a solution and it certainly doesn't explain why this is happening, BUT, this was the easiest way to prevent my game from going boom when this went wrong.

Rob-bie avatar Nov 09 '21 04:11 Rob-bie

@WolfgangSenff and @fenix-hub maybe we need to make a new case for an empty bod, but check if you are still online and soft error out? I don't think we ever planned a case if the user is trying to query something, but they made a mistake.

If this truly is an issue on android, maybe we need to also dig into that and code something special for it?

Thinking out loud here

BearDooks avatar Nov 09 '21 15:11 BearDooks

In my opinion an empty body should not be treated as an offline request in the first place, I could give it a look after work

fenix-hub avatar Nov 09 '21 15:11 fenix-hub

Here's a minimal reproducible case: https://github.com/Rob-bie/godot-firebase-firestore-bug

Rob-bie avatar Nov 09 '21 17:11 Rob-bie

@Rob-bie thanks for sending that over. Apologies I have been away. I will take a look at it and talk with @fenix-hub

Chuck

BearDooks avatar Nov 26 '21 20:11 BearDooks

@BearDooks and @fenix-hub - any updates here? I think it just fell off our plate!

WolfgangSenff avatar Oct 05 '22 21:10 WolfgangSenff