Advisory messages

[adamchalmers]

@mlfarrell says that sometimes he wants to send a message back to the frontend from the engine, saying "this endpoint is deprecated". It's not an error yet -- but it might become one. Currently the backend has no way to send that data, and the frontend has no way to show it.

Solution

• The WebSocketResponse adds a new field messages: [Message] where Message looks like {"severity": "warning", "contents": "the command Foo is deprecated"}
• The backend writes to this field
• The frontend displays them in the CodeMIrror diagnostics or wherever else is appropriate.

[jessfraz]

actually if you mark it as deprecated in rust it gets pulled thru to the openapi spec and then we render it back out as deprecated ;)

[jessfraz]

seems like a flawless approach :)

[jessfraz]

then its a compile time warning til we fix it

[lf94]

Because we're pre-v1 I think these should straight up be errors, because post-v1 launch we never want to deprecate anything.

[jessfraz]

we can make rust fail on them, its just a clippy thing

[jessfraz]

probably same for TS just less famililar

[lf94]

To be clear I agree we do need a way for the backend to tell consumers of endpoints that are gone. I just think they should be errors, not deprecation warnings.

[jessfraz]

i like doing it thru the sdks personally since we already have that functionality, if you arent using the sdks the docs will also show as deprecated

[jessfraz]

i dont think it makes sense to do it at run time since the people who use the apis likely dont want their customers to know or care about this

[jtran]

As others said, maybe there's something better we can do for deprecations that are purely internal/for us.

However, the OP proposal works well, IMO, for things that you want to surface to end-users. The thing is, I'm having trouble thinking of a specific example because it has to satisfy all these criteria:

1. LSP and interpreter would not be able to catch this
2. It's not a hard error
3. It can't be auto-fixed because of a judgment call that only the end-user can determine

[jessfraz]

im very pro using the sdks since we literally already have code that does this, all you need to do is set #[deprecated] the rust native thing on whats deprecated, then it goes into the openapi specc, then it goes into the rust lib, the js lib etc all the sdks and docs and glitter

[lf94]

(Typing from phone) Jess we may actually be digressing from the OP. I think engine just wants a way to communicate any message. Adam's proposal I believe is suitable. In retrospect my commentary was too focused on the deprecation bit.

[jessfraz]

ah thats fair, but in the case of deprecations hahahaa

[adamchalmers]

IMO we shouldn't do this unless we actually have a good reason for it. If @mlfarrell 's only use-case is deprecation, and we already have a solution for it, then let's not needlessly complicate our API.

[mlfarrell]

My use case was for warnings. I'm fine doing this as long as there's zero additional overhead in the api if the "messages" are empty. i.e. I want the fast-path to be preserved, esp considering how many of these messages i've been finding that we're sending in some of our more complex cases.

[adamchalmers]

@mlfarrell What's an example of a warning you'd like to send?

[mlfarrell]

"An Object with this UUID already exists in the scene, this will eventually become an error"

[jessfraz]

oh can we not warn and straight up error on that, i dont care if it breaks the front end its a forcing function, also i legit made it impossible for react to run execute over itself, so thats in and will get in a release, basically once this is in a PR point me at it and i can ensure we have front end in a good place

[mlfarrell]

Up to you. It could change other stuff though (i.e. I want to bring back the better scene clear method etc)

[jessfraz]

yeah i think we are shooting ourselvves in the foot down the road if we dont hard fail

[mlfarrell]

Related to something that should be flagged. Right now I log this on engine creation. afaik, only F/E tests can make this happen, but if we ever see this pop up in prod, it'd be very bad.

Error: Render thread created before previous one destroyed! This could create major problems!

[adamchalmers]

Closing because I don't think we need this yet. Let's reopen if we have a use for it.