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.

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

Uncatchable error in NodeJS Agones SDK when calling shutdown() #2304

Closed
jhowcrof opened this issue Oct 11, 2021 · 2 comments · Fixed by #2315
Closed

Uncatchable error in NodeJS Agones SDK when calling shutdown() #2304

jhowcrof opened this issue Oct 11, 2021 · 2 comments · Fixed by #2315
Labels
help wanted We would love help on these issues. Please come help us! kind/bug These are bugs.
Milestone

Comments

@jhowcrof
Copy link
Contributor

What happened:
Error logged from NodeJS Agones SDK:
Error: 14 UNAVAILABLE: Connection dropped

from watchGameServer. This happened after calling shutdown intentionally.

What you expected to happen:
I should be able to catch this error, and I cannot because it is wrapped in a callback. The error callback is executed asynchronously, so It throws into a separate execution context and crashes the node process as an unhandled exception.

Instead, watchGameServer should take an onError callback or something, so that I can handle this sort of error.

How to reproduce it (as minimally and precisely as possible):

Just call watchGameServer, then call shutdown.

Anything else we need to know?:

Environment:

  • Agones version: 1.17.0
  • Kubernetes version (use kubectl version): v1.22.2
  • Cloud provider or hardware configuration: GKE
  • Install method (yaml/helm): helm
  • Troubleshooting guide log(s):
  • Others:
@jhowcrof jhowcrof added the kind/bug These are bugs. label Oct 11, 2021
@roberthbailey
Copy link
Member

@steven-supersolid - have you run across this in your use of the nodejs SDK?

@roberthbailey roberthbailey added the help wanted We would love help on these issues. Please come help us! label Oct 13, 2021
@steven-supersolid
Copy link
Collaborator

It looks like it can occur here https://github.com/googleforgames/agones/blob/main/sdks/nodejs/src/agonesSDK.js#L130

There is a related test but it only verifies the catching of a specific error, not throwing asynchronously https://github.com/googleforgames/agones/blob/main/sdks/nodejs/spec/agonesSDK.spec.js#L274

I wonder why the same problem does not occur when using the example code but could be an edge case https://github.com/googleforgames/agones/blob/main/examples/nodejs-simple/src/index.js#L43

I think the idea of a 2nd optional callback to watchGameServer is perfect and we just call that with the error if it is present, or do nothing if it is not, removing the throw as it has no benefit. We can also remove the capture of the specific grpc.status.CANCELLED error and let the client code decide what to do with any errors.

I can make the PR within the next couple of days unless anyone else wants to? I would include:

  • Code change above
  • New test under watchGameServer describe
  • Remove test for CANCELLED errors
  • Update example code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We would love help on these issues. Please come help us! kind/bug These are bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants