Sails: No error handler for jsonp __getcookie in sails.io.js

[Lovinity]

Node version: 12,16.2
Sails version _(sails)_: 1.2.4
ORM hook version _(sails-hook-orm)_:
Sockets hook version _(sails-hook-sockets)_: 1.2.1
Organics hook version _(sails-hook-organics)_:
Grunt hook version _(sails-hook-grunt)_:
Uploads hook version _(sails-hook-uploads)_:
DB adapter & version _(e.g. [email protected])_:
Skipper adapter & version _(e.g. [email protected])_:

---

There is no error handler for the jsonp request to the server's __getcookie. This means there is no way to signal via socket events that this method failed and a connection cannot / will not be established.

An example: My sails.js server is currently in the process of renewing the SSL certificate. The current one just expired. The __getcookie request from sails.io fails with certificate invalid. However, the error is simply thrown in the console; no socket events are fired to indicate the connection failed and the socket will not connect. Therefore, I am unable to provide the user with a connection failed error screen on the web page.

The error occurs on line 517:
document.getElementsByTagName('head')[ 0 ].appendChild(scriptEl);

A try / catch block for this line does not seem to work, nor does it work where jsonp is called. This is probably because the error occurs within the scope of the created script tag.

For this reason, I don't know if any resolution can be made (that is beyond me). But it would be a lifesaver if it could.

Edit: ...come to think of it, perhaps there should be a better way to handle all cases where the jsonp ends up not calling the internal function to continue the connection (bad URL, no internet, SSL problem, server down, etc). If it fails for any reason, none of the socket event handlers are called since the socket is not initialized. Therefore, the error cannot be properly handled.

Edit2: A setTimeout is also complicated, it seems, especially since __getcookie would fail before the socket is even initialized by io (and therefore the socket event handlers cannot be used).

[sailsbot]

@Lovinity Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

• look for a workaround. _(Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)_
• tell us why this issue is important to you and your team. What are you trying to accomplish? _(Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)_
• make sure you've provided clear instructions on how to reproduce the bug from a clean install.
• double-check that you've provided all of the requested version and dependency information. _(Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)_
• read the code of conduct.
• if appropriate, ask your business to sponsor your issue. _(Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)_
• let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. _(Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)_
  

  ---

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

[nahanil]

Some food for thought.

try/catch can't work because the script is loaded asynchronously. The HTTP error will happen some time in the future after the function that inserts the script tag has completed.

_sailsIoJSConnect, and by proxy cb, will never be called because the __getcookie response doesn't return anything useful to call it so the socket connection stuff will never start (because cb never gets called).

Maybe an error handler could be added to scriptEl around here before it's inserted into the DOM for situations like these.

scriptEl.addEventListener('error', function (err) {
  alert('boom');
  console.error(err);
  // But IRL do something useful here
})

There's some vague hints on error handling dynamic script tags on MDN https://developer.mozilla.org/en-US/docs/Web/API/HTMLScriptElement#Dynamically_importing_scripts

[Lovinity]

@nahanil Sweet. Thanks for the information; I had no idea script tags had error handlers. The problem though is getting that error to be passed to the renderer javascript. This error handler exists within sails.io.js, but I somehow need to get this error passed to the javascript which initiated the socket via io.sails.connect. Since the socket isn't initialized yet, I can't emit a socket event. Is there perhaps a way to do an io.sails.on?

[whichking]

Hey, @Lovinity! To connect to the socket, are you explicitly calling connect(), or are you using auto-connect?

[Lovinity]

@madisonhicks I'm using connect() because server requires an auth query to be sent upon connection (handled by sails.js beforeConnect).

[whichking]

Thanks for your patience, @Lovinity!

Maybe we could add a new method called connectCarefully()—an async function—which, when used with await or .then() chaining, would allow that kind of error handling. What do you think?

[Lovinity]

That sounds like it could definitely work, since connectCarefully would be called in renderer, and the then() chain can provide an error, allowing me to display an error message to the user.