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

Update device error model #232

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fernandopradocabrillo
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • correction
  • enhancement/feature
  • documentation

What this PR does / why we need it:

Align error model with Commonalities for meta Spring25

Which issue(s) this PR fixes:

Fixes #230

Special notes for reviewers:

In the subscriptions APIs there is one error that I'm not sure if we have to update or not.

TokenMismatch:
   value:
    status: 403
    code: SUBSCRIPTION_MISMATCH
    message: Inconsistent access token for requested events subscription

It looks like it is the equivalent to 403 INVALID_TOKEN_CONTEXT. It may make sense to keep it since the device object is required when creating the subscription, regardless of the token type. I tend to think we should keep it.

@bigludo7
Copy link
Collaborator

bigludo7 commented Dec 6, 2024

@fernandopradocabrillo Agree with you to keep it but in order to be explicit the code should not be: INVALID_TOKEN_CONTEXT ?

@bigludo7
Copy link
Collaborator

bigludo7 commented Dec 8, 2024

Hi @fernandopradocabrillo
Additional thinking:

  • The callback must have 429 in device-roaming-status-subscriptions.yaml (in lines 140/155)

Following points are open:

  • Do we keep 500, 501, 502, 503 & 504 in the yamls ? they are not anymore mandatory to be defined
  • 429 is not mandatory for the GET /subscriptions/{subscriptionId} & GET /subscriptions.
  • same for DELETE

@fernandopradocabrillo
Copy link
Collaborator Author

fernandopradocabrillo commented Dec 10, 2024

@fernandopradocabrillo Agree with you to keep it but in order to be explicit the code should not be: INVALID_TOKEN_CONTEXT ?

I'm not sure, I see that in geofencing it is also SUBSCRIPTION_MISMATCH: https://github.com/camaraproject/DeviceLocation/blob/main/code/API_definitions/geofencing-subscriptions.yaml#L1166

do you know if there is any conversation right now in commonalities regarding the errors in subscriptions?

@akoshunyadi
Copy link
Collaborator

@fernandopradocabrillo would it be possible to cover the 2 new APIs here as well? 🙂 (connected-network-type-subscriptions is still to be merged)

@fernandopradocabrillo
Copy link
Collaborator Author

fernandopradocabrillo commented Dec 19, 2024

@fernandopradocabrillo would it be possible to cover the 2 new APIs here as well? 🙂 (connected-network-type-subscriptions is still to be merged)

yeah and I won't charge extra

@fernandopradocabrillo
Copy link
Collaborator Author

@camaraproject/device-status_maintainers shouldn't the POST /subscription endpoint also have the 404 IDENTIFIER_NOT_FOUND error?

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

Successfully merging this pull request may close these issues.

Align Device Errors with last Commonalities agreement for Spring25
3 participants