grpc-web icon indicating copy to clipboard operation
grpc-web copied to clipboard

Status Codes are shown in incorrect alphabetical ordering

Open NoMan2000 opened this issue 2 years ago • 1 comments

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

https://github.com/grpc/grpc-web/blob/1779661dded5b918cdd84a216f708b93ebf2384c/packages/grpc-web/index.d.ts#L130

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
        }

NoMan2000 avatar May 16 '22 14:05 NoMan2000

Aha interesting.. thanks for pointing this out!

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

Thanks!

sampajano avatar May 16 '22 22:05 sampajano

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.

chandraaditya avatar Sep 17 '22 10:09 chandraaditya

@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:

https://github.com/grpc/grpc-web/blob/903601a4266bdd3db2a3cb3cad78ff6f03a5d68b/net/grpc/gateway/examples/echo/ts-example/client.ts#L84-L88

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 :)

sampajano avatar Sep 19 '22 23:09 sampajano

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.

chandraaditya avatar Sep 20 '22 06:09 chandraaditya

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! 😃

sampajano avatar Sep 21 '22 21:09 sampajano