Citizen.Await seems to return nil
sometimes instead of a table at this line https://github.com/citizenfx/fivem/blob/master/data/shared/citizen/scripting/lua/scheduler.lua#L577
any repro other than āsometimesā?
Hard to debug, stack traces are not clear, so no, itās related to coroutines.
Still an issue, I canāt help because the scheduler code is satanic to me, but the people who made it could investigate this.
good job providing no additional info!
ask @Brouznouf, though from what I hear heās not doing anything FiveM-related lately, so youāll really have to provide more information as we didnāt make it.
Seriously ? Iām well aware that solving issue is difficult and I value giving most information as possible, but there is a point where it is irrelevant, this is a design flaw, not something a user of the API can probably solve. I could rewrite part of the scheduler, but that would require your approval and some guidelines and will also led to a very different code.
what is a design flaw? the function is meant to only return table in this scenario, and you provided 0 information that can repro the scenario you encounter where it doesnāt.
None of this needs a ārewriteā to resolve at all, and since you apparently know better and donāt provide any information to fix things why donāt you write āa very different codeā and PR it, so it can be reviewed?
Just checked code flow around there, thereās no way whatsoever that it can return nil - promise rejection will lead to a Lua error
being caused in the coroutine, not a nil
return value from Citizen.Await, and a truthy value will return said value.
Did you test this on the latest game version (server pipeline 822 or higher, client updater version 1126140+)?
The usage of coroutines looks wrong, leading to a lot of edge cases, but I will know more about it if I try a rewrite.
First, I donāt know what is your PR policy, so I donāt want to work for nothing, secondly I donāt know what are exactly the requirements expected from the scheduler, changes could break its usage.
There is an issue, you may not care, but I do, and it has been 8 months, so I donāt understand how pointing this out should give me those kind of messages ?
I will look into it (and will probably need answers to do so).
I can focus on just rewriting the threads managment.
Is the ācurrent threadā variables useful for something (beyond internal managment) ?
Do promises are actually required or used by people beyond yielding the coroutine ?
thereās really no PR policy afaik, whatever pleases the elements will get PRād, but an element may know more. Also nice job again evading the question and not specifying what these āedge casesā are .
You can see the idea here https://github.com/ImagicTheCat/fivem/commit/095cbb2023dbf7010bd0ca3bd94ce1d8b3a95122.
Itās without optimization.
I donāt know how to handle the promise stuff (because I donāt use it and donāt understand the concept), and canāt really test it because my resources depend on Await.
Concept makes sense but doesnāt really seem necessary to fix any bugs - yeah it makes internal bookkeeping easier but this bug doesnāt seem triggered by internal bookkeeping at all.
Also yes, promises are an exposed API and definitely need to be implemented correctly, thereās actually people using this.
Incredibleā¦ Thatās actually the problem, they seem to not be implemented/used properly because of the previous complexity, there is probably a premature promise solved check that causes returning a nil value. I donāt use promises, but the part in comments probably needs to handle two cases:
- if already solved before yield, return value
- if not already solved, yield and handle promise event for resume
Anyway, there is a lot of global variables in the original version, itās expected to have random issues.
Yes, but the same for rejecting and throwing an error. error
needs to be thrown within the target coroutine, as well, to allow pcall
to catch it.
That was not the point of my changes, just a secondary effect. This way is more straightforward, you only resume what you yield, so other stuff using coroutines which are following this rule can all work together.
Hey,
Iām actually using the promise / await system without any problem (and with vrp, but an old version), so itās hard to understand whatās wrong here. However i do not have early returning promise so it may be because of that.
For me the only flaw was having this lib was to returning a nil value on error, but it seems to have been fixed by last commits.
It may come from the promise implementation, the current one is from https://github.com/zserge/lua-promises and there seems to be issues with some errors not being rejected correctly
The promise stuff is necessary in order to have a consistent API for the Await API (no random callback definition). Another API may be used but this is a very common standard that a lot of people know so i donāt see the point of reinventing the wheel here.
I carefully recopied the Citizen.Await function to keep itās actual behavior, while reworking the coroutines.
It seems to solve the issue (which wasnāt expected), but even if itās not, now itās possible to properly use the coroutines without relying on Citizen.Await and promises. Everyone wins. No idea about performance, but I donāt have anything significant to benchmark.
The PR: https://github.com/citizenfx/fivem/pull/157
I still think promise are a bad thing coming from a world without coroutines, especially in Lua. But here is the compromise.
I agree that callback / promise are not needed, however there is a ton of library using callback system, having promise + await thing allow a smooth transition
Also very few people actually understand how coroutines works so it allows them to use callback while still allowing other people like you and me to not use them, after all itās only a question on how to write of code, result is the same.
But that was not the case before my fix, I was forced to use Citizen.Await and promises because of the way coroutines are managed.
Also, the issue is not punctual/random anymore, Citizen.Await will constantly fail without this fix with vRP 2 (my project, dev branch) when the DB driver is registered, so you actually can reproduce it.