dcs: Remap error codes from TTJS

[halimi]

Summary

Closes #7045

HTTP error codes coming from the JS passed through to the client. These error codes are not relevant to the clients and sometimes they are confusing e.g. wrong owner token results 403 forbidden but it should be just 400 bad request. To not confuse the client we remapped some of the error codes.

Changes

This change remaps the error code from TTJS.

• HTTP error code 400 bad request and 401 unauthorized to 500 internal server error
• HTTP error code 403 forbidden to 400 bad request

Testing

Steps

Try to register a device with a wrong authentication code and you should get error 400 bad request instead of 403 forbidden.

Results

Registering a device with wrong authentication code results error code 400.

Regressions

Some of the error codes have been changed.

Notes for Reviewers

Checklist

• [ ] Scope: The referenced issue is addressed, there are no unrelated changes.
• [ ] Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
• [ ] Documentation: Relevant documentation is added or updated.
• [ ] Testing: The steps/process to test this feature are clearly explained including testing for regressions.
• [ ] Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
• [ ] Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
• [ ] Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

[halimi]

@johanstokking you are right, I will change it to check the error message. Is the error message claim failed with given owner token in the proposal correct? I cannot find it in the code.

[johanstokking]

@johanstokking you are right, I will change it to check the error message. Is the error message claim failed with given owner token in the proposal correct? I cannot find it in the code.

It comes from here: https://github.com/TheThingsIndustries/lorawan-join-server/blob/master/src/core/controller.ts#L486-L488.

The response will be a JSON message {"message":"claim failed with given owner token"} with status code 403. That needs to be mapped to 400 bad request. The others should become 500. So check if the Content-Type is application/json, then unmarshal JSON into a struct with Message stringjson:"message"` and then check the string. Ideally add a comment on the aforementioned line in TTJS to not change the error message because clients depend on it.

A small nit comment is that errBadRequest should really be InvalidArgument to make it clearer for our future selves. Because Internal gets mapped to 500 via gRPC Web, meaning that returning errBadRequest in code results in 500 for the client. That is hard to debug.

[halimi]

@johanstokking thank you for clarifying what is the join server and where the error comes from. Now it make more sense. The error message is already unmarshaled so now I'm checking the message and based on that returning back with a different error code.

I agree to name an internal server error as errBadRequest is a bad idea and makes our life harder later, I've changed it back and created a new errInternalError.

[KrishnaIyer]

@halimi: For future reference, we don't use Closes for issues, unless they don't require testing and/or are small fixes. this way the issue stays open and we only close it after it's tested in staging.

[halimi]

@halimi: For future reference, we don't use Closes for issues, unless they don't require testing and/or are small fixes. this way the issue stays open and we only close it after it's tested in staging.

Understood, I will keep in mind, thank you.