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

Status Codes are shown in incorrect alphabetical ordering #1243

Closed
NoMan2000 opened this issue May 16, 2022 · 5 comments
Closed

Status Codes are shown in incorrect alphabetical ordering #1243

NoMan2000 opened this issue May 16, 2022 · 5 comments
Labels

Comments

@NoMan2000
Copy link

I don't know why, but the StatusCode enums are in the wrong order.

export enum StatusCode {

That shows them sorted alphabetically, but they are not actually that way at all. The actual ordering:

StatusCode: {
          OK: 0,
          CANCELLED: 1,
          UNKNOWN: 2,
          INVALID_ARGUMENT: 3,
          DEADLINE_EXCEEDED: 4,
          NOT_FOUND: 5,
          ALREADY_EXISTS: 6,
          PERMISSION_DENIED: 7,
          UNAUTHENTICATED: 16,
          RESOURCE_EXHAUSTED: 8,
          FAILED_PRECONDITION: 9,
          ABORTED: 10,
          OUT_OF_RANGE: 11,
          UNIMPLEMENTED: 12,
          INTERNAL: 13,
          UNAVAILABLE: 14,
          DATA_LOSS: 15
        }
@sampajano
Copy link
Collaborator

Aha interesting.. thanks for pointing this out!

I'm curious if it causes any build time / runtime issues? Or any other usability issues?

Thanks!

@chandraaditya
Copy link
Contributor

Yes it is causing a lot of issues, I did not notice this until now, but I have a check for grpcWeb.StatusCode.NOT_FOUND, it is code 8 not 5 on grpc-web, therefore when my backend is sending code 5 it is being interpreted as FAILED_PRECONDITION instead of NOT_FOUND which is the actual code.

@sampajano
Copy link
Collaborator

sampajano commented Sep 19, 2022

@chandraaditya thanks so much for your fix!

Yes it is causing a lot of issues, I did not notice this until now, but I have a check for grpcWeb.StatusCode.NOT_FOUND, it is code 8 not 5 on grpc-web, therefore when my backend is sending code 5 it is being interpreted as FAILED_PRECONDITION instead of NOT_FOUND which is the actual code.

Also thanks for the comment! Although I'm wonder if this will actually cause a runtime issue or not?

As far as i understand, if you have a check in .ts file that looks like if (statusCode == StatusCode.NOT_FOUND), similar to what we have in our Typescript demo:

if (err && err.code !== grpcWeb.StatusCode.OK) {
EchoApp.addRightMessage(
'Error code: ' + err.code + ' "' + decodeURI(err.message) +
'"');
}

Then when tsc compiles the TS file, the same code will be generated as Javascript, and it would work. Is that right? (Or maybe an advanced tsc mode will directly output the enum number rather than StatusCode.NOT_FOUND?)

If so, then I guess this issue will be causing some confusion during understanding the error (when referring to the index.d.ts file), but not causing a runtime issue?

Thanks in advance :)

@chandraaditya
Copy link
Contributor

Not a problem!

So I'm using NextJS and I'm not entirely sure how it compiles TS and if it does some weird stuff.

But essentially what was happening was I needed to check if the response was NOT_FOUND so I did rpcError.code == grpcWeb.StatusCode.NOT_FOUND, and that was for sure causing errors during runtime.

Because even though my server is sending the NOT_FOUND response which is error code 5, rpcError.Code was actually interpreted as FAILED_PRECONDITION which is error code 5.
So during the check in the IF statement, rpcError.code is 5 on the left side, then on the right side grpcWeb.StatusCode.NOT_FOUND is actually 8 so that condition was failing and my code was never executed even though it is expected to pass.

But apart from the fact that my code was not working, I'm not 100% sure what the reason was, I do remember trying to console.log the if condition and it showing 5 and 8 which is when I switched from grpcWeb.StatusCode.NOT_FOUND to just 5 like so:
rpcError.code.valueOf() === 5
And it started working.

@sampajano
Copy link
Collaborator

Oh wow thanks for the very detailed explanation! It's very interesting to know! 😃

I'm not familiar with NextJS so maybe it's somehow directly compile the TS enum into numbers (unlike the TS example we have, which pretty much generated the same JS code and was working fine for us.)

We'll definitely keep this difference in mind going forward, and will be careful about future enums!

Thanks a lot for your report and code fix! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants