fix: NPE when connectionCookie is undefined
#13541
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Unable to find test scripts. Please add necessary tests to the PR. |
app/rts/src/server.ts
Outdated
| log.error("Error authenticating", error); | ||
| } | ||
| return false; | ||
| if (connectionCookie == null || connectionCookie === "") { |
There was a problem hiding this comment.
=== for the first check.
There was a problem hiding this comment.
That was the problem actually. If we use === for null, then undefined will go through. I want to check for null and undefined. It is recommended to use == for null-checks in Javascript, and === everywhere else. Like the smart option in eslint: https://eslint.org/docs/rules/eqeqeq.
There was a problem hiding this comment.
One quick check you can deploy against null, undefined and "" is !connectionCookie. This is okay as long as you are not expecting a 0 or false.
I rather have very boring and explicit code base, like connectionCookie === undefined || connectionCookie === null || connectionCookie === ""; it tells me exactly when we going to drop in the if block.
connectionCookie == null || connectionCookie === "" needs a context and cursorily looks like one = is missing.
Please make it explicit.
There was a problem hiding this comment.
@sharat87 This change is pending. For maintainability it is very important for code to be unsurprising and boring.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Fix NPE in RTS when the
connectionCookieisundefined.