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

Si abro dos veces el repl, la segunda no puede abrir el diagrama dinámico porque el puerto ya está usado #58

Closed
JuanFdS opened this issue May 29, 2023 · 11 comments
Labels
componente: consola Interpreta los comandos de la consola enhancement New feature or request

Comments

@JuanFdS
Copy link

JuanFdS commented May 29, 2023

Wollok cli intenta abrir ambas veces en el puerto 3000 y la segunda falla con:

✗ File c:/Users/juanm/OneDrive/Documentos/Developmeectos/facu/pdep/Wollok/prueba_file.wlk doesn't exist or is outside of project!
✓ Object diagram available at: http://localhost:3000
wollok> node:events:505
throw er; // Unhandled 'error' event
at processTicksAndRejections (node:internal/process/task_queues:83:21) {
code: 'EADDRINUSE',
errno: -4091,
syscall: 'listen',
address: '127.0.0.1',
port: 3000
}

Como reproducir:

  • correr el repl de wollok cli en una terminal
  • una vez abrió, correr el repl de wollok cli en otra
@JuanFdS JuanFdS changed the title Si abro dos veces wollok cli, la segunda no puede abrir el diagrama dinámico porque el puerto ya está usado Si abro dos veces el repl, la segunda no puede abrir el diagrama dinámico porque el puerto ya está usado May 29, 2023
@PalumboN PalumboN added bug Something isn't working componente: diagrama-dinamico Muestra los objetos en el diagrama labels Jun 17, 2023
@PalumboN
Copy link
Contributor

Y cuál sería el comportamiento esperado?

  • Podríamos hacer el checkeo y si no intentar con otro (3001?).
  • Tirar un error bonito diciendo que el puerto está usado, y que se fije si ya no tiene un Wollok corriendo (me imagino a un estudiante recién iniciado que se puede confundir teniendo muchas sesiones abiertas).

Un poco de cada?

@fdodino
Copy link
Contributor

fdodino commented Sep 29, 2023

@PalumboN @JuanFdS en este PR hice que cuando ejecutás el REPL mate todas las sesiones abiertas hasta el momento (igual que pasaba en Xtext) para que no queden puertos abiertos (me parece que para una persona que arranca hacer eso le puede traer problemas de performance y la sensación de que "hay que apagar la compu porque Wollok consume mucha memoria"). Cierro el ticket a menos de que quieras hacer otra cosa.

@fdodino fdodino closed this as completed Sep 29, 2023
@PalumboN
Copy link
Contributor

Holis! Ese PR sirve para el VSCode, pero qué pasa si uso la CLI por consola?

@fdodino
Copy link
Contributor

fdodino commented Sep 29, 2023

Buen punto. Reabro....

@fdodino fdodino reopened this Sep 29, 2023
@fdodino
Copy link
Contributor

fdodino commented Oct 1, 2023

@PalumboN , qué te parece "un poco de ambas":

  • por defecto, que muestre un error amable en lugar del ADDRINUSE
  • pero que vos lo puedas forzar (con un --new) para que efectivamente te abra otro puerto libre

@fdodino fdodino added componente: consola Interpreta los comandos de la consola enhancement New feature or request and removed componente: diagrama-dinamico Muestra los objetos en el diagrama bug Something isn't working labels Oct 1, 2023
fdodino added a commit that referenced this issue Oct 2, 2023
@fdodino
Copy link
Contributor

fdodino commented Oct 2, 2023

En realidad, el comportamiento sería

  • por defecto, mostrar un error amable, onda
    image

  • y si querés usá la opción --port indicando un puerto que esté disponible:
    image

@PalumboN @JuanFdS , qué les parece?

@PalumboN
Copy link
Contributor

PalumboN commented Oct 2, 2023

Me parece bien lo del checkeo y la opción del port 👍

Hago estas preguntas / comentarios:

  1. Si el puerto está ocupado, te dejo usar la consola igual o la mato?
  2. En la imagen que pasaste, aún si el puerto está ocupado dice que ✅ el diagrama está disponible :P
  3. En el error / warning yo pondría
    "⚠️ No se pudo levantar el diagrama dinámico porque el puerto 3000 está ocupado. Asegurate de no tener otro REPL abierto en otra consola. Si querés levantar el diagrama en otro puerto, utilizá la opción..."

@fdodino
Copy link
Contributor

fdodino commented Oct 2, 2023

Si el puerto está ocupado, te dejo usar la consola igual o la mato?

Yo diría de matarlo, porque dejar dos consolas va a ser para que se arme lío o reporten errores que no son tales.

En la imagen que pasaste, aún si el puerto está ocupado dice que ✅ el diagrama está disponible :P

Sí, eso me molesta, pero todavía no pude ver cómo hacer para detectar de que hubo un error (en realidad supongo que deberíamos ver qué eventos hay para mostrar ese mensaje una vez que efectivamente se conectó).

En paralelo, hay un mensaje que me resulta molesto y es que como usamos socket, cada dos por tres se desconecta y se conecta y eso ensucia la consola. Yo lo que haría es que cuando se desconecte por más de 3 segundos ahí se muestre el mensaje, con lo cual tiraría un setTimeout...

En el error / warning yo pondría
"⚠️ No se pudo levantar el diagrama dinámico porque el puerto 3000 está ocupado. Asegurate de no tener otro REPL abierto en otra consola. Si querés levantar el diagrama en otro puerto, utilizá la opción..."

Dale!

@PalumboN
Copy link
Contributor

PalumboN commented Oct 2, 2023

Yo no loguearía nada cuando se conectan al diagrama... Como muuuucho lo dejaría para un modo DEBUG

fdodino added a commit that referenced this issue Oct 2, 2023
fdodino added a commit that referenced this issue Oct 2, 2023
@fdodino
Copy link
Contributor

fdodino commented Oct 3, 2023

Uhhh... datos de cosas que me pasaron:

  1. no logro que el mensajito "Dynamic diagram available..." se muestre solo cuando te conectás, o lo hace una sola vez (incluso cuando te conectás a otros puertos) o lo hace siempre (aun cuando tira error). Voy a seguir investigando.
  2. Es gracioso que no me funciona en el navegador el diagrama dinámico para otro puerto distinto del 3000, porque...
        import { io } from "./lib/socket.io.esm.min.js";

        const socket = io("http://localhost:3000");
        socket.on("connect", function () {

está hardcodeado el 3000 al server... (si te conectás al 3001 con la segunda instancia la página responde pero hay un error de CORS)

Voy a seguir investigando

fdodino added a commit that referenced this issue Oct 3, 2023
fdodino added a commit that referenced this issue Oct 3, 2023
@fdodino
Copy link
Contributor

fdodino commented Oct 3, 2023

Listo el pollo, arreglé las dos cositas.

@fdodino fdodino closed this as completed in 7d58786 Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
componente: consola Interpreta los comandos de la consola enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants