Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

recursive alias causes error #1647

Open
Mathekatze opened this issue Sep 8, 2024 · 15 comments · May be fixed by #1649
Open

recursive alias causes error #1647

Mathekatze opened this issue Sep 8, 2024 · 15 comments · May be fixed by #1649
Labels
bug Something isn't working

Comments

@Mathekatze
Copy link

version of the game: Bitburner v2.6.2 (633da38)

I have aliased buy="buy -l". Now, when I type "buy" into the terminal and press enter, the game hangs briefly, then gives the following error:

UNCAUGHT PROMISE ERROR
You forgot to await a promise
maybe hack / grow / weaken ?

RUNTIME ERROR:

too much recursion
stack:
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18541
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658
h@https://bitburner-official.github.io/dist/main.bundle.js:2:18658

bitburnerSave_1725813539_BN1x1.json.gz
@d0sboots
Copy link
Collaborator

d0sboots commented Sep 8, 2024 •
edited
Loading

I'm not quite going to mark this WONTFIX, but the behavior is unlikely to change here. Apparently we expand aliases recursively (unlike bash); this error is not the nicest for if that forms a loop, but it does explain the issue. :)

@catloversg
Copy link
Contributor

I remember seeing this bug reported a couple of months ago. Back then, I thought about writing a detectRecursiveAlias to detect this problem and print a useful error message to the terminal instead of letting it throw an "too much recursion" error. When I asked for the specific expected behavior of some test cases, Hoekstraa said something along the lines of "look at POSIX spec". To be honest, that's not useful at all. In the end, I scrapped the plan and forgot about this problem. There are problems with our "alias" feature:

  • Even without our specific "global alias", making it POSIX-compliant is not simple. We tried to make it POSIX-compliant, and we failed everytime.
  • When we try to incorporate our "global alias", things get even more complicated.

I tried to check old issues/PRs/discussions (on Discord), read the POSIX spec, test it on bash, write test cases, decide the expected behavior, think about the potential change in the behavior, etc. Things got messy really fast. I think there are 2 ways to solve it:

  • Spend a significant amount of effort to make it POSIX-compliant. As I said before, this task is really hard. It's not as simple as looking at some pseudocode and converting it to JS code. Even when we do it, we still have the problem of the global alias. There is nothing like that in real life, so we still have to make up some rules. In the end, I doubt it will ever be POSIX-compliant. [1]
  • Detect the recursive alias and print an error message. The detection does not need to be perfect. As long as it catches common use cases that lead to the infinite recursion, it's good. We can improve it over time. [2]

@d0sboots What do you think about [2]?

@d0sboots
Copy link
Collaborator

d0sboots commented Sep 9, 2024

2 is basically what we already have, just with a bad error message. XD

If you want to catch the exception and try to detect if it was a stack overflow, go ahead.

@catloversg
Copy link
Contributor

I don't think we have a working detection of infinite recursion. In applyAliases (src\Alias.ts), we check currentlyProcessingAliases, but that check is flawed.

@d0sboots
Copy link
Collaborator

d0sboots commented Sep 9, 2024

Maybe you misunderstood me. All known JS engines have working detection of infinite recursion. You can see that detection functioning in the bug report here, via the pasted exception.

All that remains is to show a better message for this case.

@catloversg
Copy link
Contributor

Oh, I got it now. I thought about implementing a custom "detector" for our alias/global alias, but catching the JS error is a good idea.

@StormSurge95
Copy link

StormSurge95 commented Sep 11, 2024 •
edited
Loading

I know nothing about POSIX compliance...but why not simply add the recursive alias info into the help text for alias and then have something along the lines of:

if (aliasValue.includes(` ${aliasKey} `)) showRecursiveAliasError()

@d0sboots
Copy link
Collaborator

To the second part, because the recursion can happen through more than one alias.

@cmfrydos
Copy link
Contributor

cmfrydos commented Sep 12, 2024 •
edited
Loading

You'd keep track of every rule already applied and don't apply a rule twice. Also implementing the posix rule that only unquoted words are potentially replaced by an alias could help here, since this would also allow writing an alias in the style TO tried to do.

I doubt js really has an infinite recursion detection algorithm, since that would solve the halting problem. The most it probably does is to stop when the function call stack overflows. That's why TO experience a slight lag before the error was thrown.

@cmfrydos
Copy link
Contributor

cmfrydos commented Sep 12, 2024 •
edited
Loading

wait, keeping track of the aliases should already be in: #630
and #741. So this might just need a second look.

@cmfrydos cmfrydos linked a pull request Sep 12, 2024 that will close this issue
Open
@cmfrydos
Copy link
Contributor

cmfrydos commented Sep 12, 2024 •
edited
Loading

I have fixed a small flaw in the recursive check for alias application. However, I have not modified the function to ensure only unquoted commands are aliased, as I am still reviewing the POSIX quoting rules and how they are interpreted by the game's terminal.

Additionally, I recommend not attempting to catch the "too much recursion" exception. This error varies by browser, leading to potential inconsistencies (more details here: MDN - InternalError: too much recursion).

The function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases: string[] = []) already effectively tracks the recursion depth and could be used for this purpose. This approach was previously replaced by a more advanced check introduced in #741. The old depth check could be reinstated as a fallback.

Please review my change and confirm if the new recursion check aligns with your expectations.

@cmfrydos
Copy link
Contributor

I know nothing about POSIX compliance...but why not simply add the recursive alias info into the help text for alias and then have something along the lines of:

if (aliasValue.includes(` ${aliasKey} `)) showRecursiveAliasError()

This would prevent an alias definition like
buy="buy -l" entirely.
I believe the alias system was never supposed to throw exceptions, but should just stop substituting when it runs into a recursion.

@gmcew gmcew added the bug Something isn't working label Oct 20, 2024
@bcox
Copy link

bcox commented Oct 22, 2024

It might make sense for an alias only be able to call a 'base' command.
alias scan=scan-analyze // scan from the command line runs scan-analyze.
alias connected=scan // connected runs the base command scan, not the alias.
That should get rid of the recursion mess, unless I am missing something.

@cmfrydos
Copy link
Contributor

The recursion is fine now; I fixed a small flaw in #1649. However, I noticed another issue just before merging, but I didn’t have time to address it last month. I'll have some free time at the start of next week and can finish it then. Apologies for the delay.

@stoltzld
Copy link

I know nothing about POSIX compliance...but why not simply add the recursive alias info into the help text for alias and then have something along the lines of:

if (aliasValue.includes(` ${aliasKey} `)) showRecursiveAliasError()

This would prevent an alias definition like buy="buy -l" entirely. I believe the alias system was never supposed to throw exceptions, but should just stop substituting when it runs into a recursion.

Is there a reason to allow recursion by default? Why not add a flag to alias to allow processing recursion and don't do recursion otherwise....I could see how aliasing a command to a command with a specific option would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

FIX: Resolve Alias Recursion Issue by Preventing Duplicate Rule Application cmfrydos/bitburner-src
8 participants
@d0sboots @bcox @stoltzld @StormSurge95 @cmfrydos @gmcew @Mathekatze @catloversg