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

Dedicated document for Rejection Reason Codes #2742

Open
1 task
maxsharabayko opened this issue May 24, 2023 · 11 comments · May be fixed by #2762
Open
1 task

Dedicated document for Rejection Reason Codes #2742

maxsharabayko opened this issue May 24, 2023 · 11 comments · May be fixed by #2762
Labels
[docs] Area: Improvements or additions to documentation Type: Maintenance Work required to maintain or clean up the code
Milestone

Comments

@maxsharabayko
Copy link
Collaborator

maxsharabayko commented May 24, 2023

Currently rejection reason codes are described in several places.

Better to have a separate document, especially given that more might be defined further and a more extensive use may be expected (e.g. see #1292, #2638).

TODO

Create a dedicated document in docs/API/rejection-codes.md and compile the codes from both places mentioned above.
Add references to the new document.

@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [docs] Area: Improvements or additions to documentation labels May 24, 2023
@maxsharabayko maxsharabayko added this to the v1.6.0 milestone May 24, 2023
@jeandube
Copy link
Collaborator

SDP/SRT

@ethouris
Copy link
Collaborator

This separation is intended.

The rejection codes in API-functions.md define the "system" rejection codes, that is, erroneous situation caused by SRT-specific settings or a general situation. They may occur in a normal SRT operation without any participation of the application, as well as the application has no possibility to simulate these codes. Also the description of the rejection code is a description of a situation that did happen.

Those in "Access Control" are codes intended for use by the application to forcefully reject the connection in the listener callback. This is only a guideline and standard proposal, and if an application decides to use them, it must still adopt these codes and publish its specific description of them. The description in the documentation is only an example or proposal, not an explanation of the code's meaning because only the application that adopts this code can define its true meaning.

Even though the code can be read in one place as coming from both sources, these two sources are strictly separate.

As per expansion, these parts are also separate - system rejection codes would be added as features of SRT get modified, while application rejection codes could expand as SRT applications get in better communication with one another and the need for additional codes appear by that reason.

@stevomatthews
Copy link
Collaborator

I don't think a separate document is necessary. We can just add a note to API-functions.md, edit the existing description in access-control.md, and then add cross-reference links between them. One specific thing that is not clearly explained is the difference between the prefixes SRT_REJ, SRT_REJC, and SRT_REJX.

@maxsharabayko
Copy link
Collaborator Author

Take a look at this from the user's point of view.
Imagine you are working with the srt_getrejectreason(..) function. The description of the function is located in the API-functions.md together with SRT specific rejection reason codes in the range 0 - 99 (SRT_REJ_*).
The description provides no clue that the rejection code might also be SRT_REJX_* described in the access control guidelines.
Furthermore, now I need to have two documents open to interpret or find a proper rejection reason code instead of having a combined table and a single document. that could be referenced from all those places where need.
That's without saying that rejection codes description complicate both documents (API-functions and Access Control), because instead of understanding the topic through a concise description you also get a piece of extra information with a list of rejection codes you didn't intend to read about.

@ethouris
Copy link
Collaborator

I understand that reasoning, but then collecting all the codes in one document would be misleading.

If you are an application developer and you use the srt_getrejectreason function, you want to interpret the code, then all you need is a list of SRT system rejection codes - because there's no possibility that the system could return you anything else.

However, this function may return you just as well application rejection codes that have been passed from the peer application. The developer might want to look into some documentation to understand the code. But then, the explanation provided by the documentation doesn't tell you the exact meaning. The true meaning of the code is what the peer application defines for itself and the explanation should be searched in THIS APPLICATION's documentation, not in SRT documentation. The description in the SRT documents for the SRT_REJX_* codes has a completely different purpose - it's a proposal of what situations may be reported by the application, it's for the application so that they copy it into their own documentation, modify a bit to adjust to their need, delete those that they don't use and publish it.

If there is a situation that the peer application didn't make their rejection code description available, the explanation may be wanted to be searched through the list of SRT_REJX_* codes, but the user should be also made aware that this is only a hint/proposal description of a hypothetic situation intended for this code, but not necessarily for what really happened.

Because of all these things, I think the best approach would be to clearly mention in the API-functions.md file that the SRT_REJ_* codes are not the only ones that can be reported by srt_getrejectreason function and the initial text portion of the "Rejection Codes" section in access-control.md file should be moved to API-functions.md file, as actually what is described there is a part of the API (especially regarding the code value ranges used for the codes). But the list of the codes itself should, I think, remain in access-control.md file, as all things described there belongs to the same category.

@maxsharabayko
Copy link
Collaborator Author

As you say, there are SRT-internal codes (0-99).
In addition, we reserve the range 100 - 599 for rejection (status) codes defined in HTTP, even though not all of them are adopted. Those that are adopted are defined in acess_control.h. User applications are not allowed to define custom rejection codes in this space.
Any custom application codes must be defined in the range of 600-900.

  • 0 - 99: Reserved for unique SRT-specific codes (unused by HTTP)
  • 100 - 399: Info, Success and Redirection in HTTP, unused in SRT
  • 400 - 599: Client and server errors in HTTP, adopted by SRT
  • 600 - 999: unused in SRT

Even though I see certain advantages of having those definitions in two different documents, still I am concerned it is not as user-friendly as it could be if it was a single document. For me, all the possible rejection reasons is a separate big topic that deserves a dedicated and well-structured document, also clearly stating the difference between SRT-specific, SRT-reserved, and application space codes.

@maxsharabayko
Copy link
Collaborator Author

maxsharabayko commented Jun 1, 2023

After some review, here is the summary for rejection codes.

SRT_REJC_INTERNAL = 0, SRT_REJC_PREDEFINED = 1000 and SRT_REJC_USERDEFINED = 2000 mark the borders of the three main ranges.

1. SRT Internal Rejection Reasons

Defined in srt.h, reserved range 0 - 999 (below SRT_REJC_PREDEFINED).

Provides the reason why a connection is rejected by SRT.

Naming: SRT_REJ_*.

  • SRT_REJ_UNKNOWN = 0
  • SRT_REJ_SYSTEM = 1
  • ...
  • SRT_REJ_CRYPTO = 17

2. Extended Rejection Codes

Defined in access_control.h. Reserved range 1000 - 1999 (SRT_REJC_PREDEFINED - SRT_REJC_USERDEFINED).

Standard server error codes including those adopted from HTTP.
Provides the reason why the application rejects a connection. The value is expected to be set by an application via the listener callback if it wants to reject an incoming connection request.

Subranges (1000 + value):

  • 0 - 99: Reserved for unique SRT-specific codes (unused by HTTP)
  • 100 - 399: Info, Success and Redirection in HTTP, unused in SRT
  • 400 - 599: Client and server errors in HTTP, adopted by SRT
  • 600 - 999: unused in SRT

Naming: SRT_REJX_*.

Example:

  • SRT_REJX_KEY_NOTSUP (1001): The key used in the StreamID keyed string is not supported by the service.
  • SRT_REJX_BAD_REQUEST (1400).
  • ...

3. User Defined Rejection Codes

Reserved range 2000 - 2999 (higher than SRT_REJC_USERDEFINED).
Any value from this range can be defined by an application. It would be a custom code, not adopted by other vendors.

@ethouris
Copy link
Collaborator

ethouris commented Jun 1, 2023

Yeah, very good. Changes in the access control docs would be then probably not required.

@stevomatthews
Copy link
Collaborator

@maxsharabayko Lines 563 to 568 in srt.h mention SRT_REJ_E_SIZE and SRT_REJ__SIZE. Not sure what is going on here. Could you please elaborate?

@ethouris
Copy link
Collaborator

ethouris commented Jun 2, 2023

SRT_REJ_E_SIZE is correct, the other is an outdated name. This should be fixed.

@maxsharabayko
Copy link
Collaborator Author

@stevomatthews
SRT_REJ__SIZE is the old definition, to be removed in v1.6.0.

// Planned deprecation removal: rel1.6.0.
#define SRT_REJ__SIZE SRT_REJ_E_SIZE

@maxsharabayko maxsharabayko linked a pull request Jul 13, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[docs] Area: Improvements or additions to documentation Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants