BUG: Incorrect WebSocket status code 1005 description, missing status code 1006

[gc-ss]

Discussed in #1577

^{Originally posted by vbsd April 7, 2022}
It seems that there is a mistake in starlette's WebSocket status code list.

Status code 1005 has the name 1005_ABNORMAL_CLOSURE and 1006 is missing altogether. However, Mozilla MDN docs indicate that 1005 actually means "no status received". It is status code 1006 that means "abnormal closure".

It looks like what should have been

WS_1005_NO_STATUS_RECEIVED = 1005
WS_1006_ABNORMAL_CLOSURE = 1006

got mashed together and became just one line:

WS_1006_NO_STATUS_RECEIVED = 1005

Making starlette backwards-incompatible over this wouldn't make much sense, but it seems reasonable to at least add the two correct lines and document WS_1005_ABNORMAL_CLOSURE as deprecated/wrong.

[Kludex]

Did you format the message to create an issue based on the discussion or the "turn into issue" button is available to everyone?

[Kludex]

But you're right. It's an issue.

It's quite sensitive tho...

[gc-ss]

@Kludex I really appreciate you taking your time to read the details and reflecting here!

Happy to provide a PR - but are PR's merged in within a few days after there are no issues with the PR?

For example, it's unclear to me what this PR needs to move forward: #1535

[Kludex]

That PR is huge, and there was no discussion about it. Some member will check it at some point, maybe I'll be the one.

About this one, we just need to see a solution before implementing it. Changing the values is a breaking change, so I wonder if a rename with the right names and a deprecation on the old variables would be a possibility... Not sure what is the best approach here. Suggestions are welcome.

[gc-ss]

Suggestions are welcome

Understood

I wonder if a rename with the right names and a deprecation on the old variables would be a possibility

We don't want to break existing code - do we? This would be technically correct but practically, not preferable

If not, then we continue supporting the previous ambiguity but provide new/right names while updating documentation with a note.

[Kludex]

Current behavior is wrong, I don't think we should support it. I would like to deprecate, and eventually remove. That if we choose to give other names to new variables...

If we choose the same name, then there is no thinking. It's straightforward breaking change.

Do we have an alternative besides the above solutions?

[gc-ss]

Current behavior is wrong, I don't think we should support it. I would like to deprecate, and eventually remove

Agreed! This would be technically correct and reduce future headaches

If we choose the same name, then there is no thinking. It's straightforward breaking change.

Agreed

Do we have an alternative besides the above solutions?

Not anything that's simpler.

Is there a way to give a "heads up" to people/code who use the current (flawed/misleading) design (whose code will break once we merge this PR in)? in a way that would be less disruptive to them?

Eg: Do we make a post on a forum/chat?

[Kludex]

My messages above doesn't make any sense, as the variable name needs to change. I've created #1580.