Commit 077f4781 authored by vertighel's avatar vertighel
Browse files

Fase 5 JS punti 2+3: withBusyButton + fetchWithTimeout



- ui.js: nuovi helper withBusyButton(el, fn) (disabilita l'elemento
  per la durata di fn; spinner solo sui <button> veri, checkbox/select
  restano solo disabled) e fetchWithTimeout(url, options,
  timeoutMs=12000) (AbortController, nessun fetch() nel codice ne
  aveva uno).
- Dedup delle 3 copie hand-rolled del pattern busy-button: actions.js
  (.btn-universal), control.js (btn-check-target), guider-panel.js
  (postGuider).
- Applicato a tutti gli handler confermati non protetti dal documento
  delle convenzioni: Expose (control.js, un withBusyButton per
  bottone stazione via e.currentTarget), Loop toggle (loop-toggle.js,
  solo il change handler -- il poll a 5s di refresh() resta com'era,
  idempotente), stage relative (control.js -- i bottoni +/- si
  disabilitano anche a vicenda, non solo se stessi, per la race
  read-then-write segnalata esplicitamente), select-universal
  (actions.js), webcam move/preset (webcam.js), sequencer
  save/delete/refresh (sequencer.js -- deleteOB guadagna anche un
  try/catch che non aveva).
- Eccezione deliberata: postExpose usa withBusyButton ma fetch()
  normale, non fetchWithTimeout -- /api/sequencer/run blocca lato
  server finche' tutta la sequenza non finisce (sequencer.py:execute),
  puo' essere minuti per un'esposizione reale. Verificato che loop/
  guider-start/stage-position invece ritornano subito, quindi per
  quelli il timeout di default e' sicuro.
- Verificato con node --check su tutti i file toccati.
- PLAN.md aggiornato.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent ca6e40db
Loading
Loading
Loading
Loading
+50 −2
Original line number Diff line number Diff line
@@ -456,13 +456,61 @@ Working point by point per the user's request, checking in after each
      item 8 below (needs the function itself deleted too, not just
      the shim). Verified with `node --check` on all 3 touched JS
      files and a Jinja parse check on all 4 touched HTML pages.
- [ ] Extract `withBusyButton(btn, fn)` helper (dedupe the 3 existing
- [x] Extract `withBusyButton(btn, fn)` helper (dedupe the 3 existing
      copies), apply to the confirmed-unprotected click/change
      handlers (Expose, Loop toggle, stage relative, select-universal,
      webcam move/preset, sequencer save/delete/refresh) — see
      `dev/conventions/javascript.md` for the full list.
- [ ] Pair the above with an `AbortController` timeout (~10-15s) on
- [x] Pair the above with an `AbortController` timeout (~10-15s) on
      every `fetch()` — none exist today.

      Did both together (touching the same call sites either way).
      `withBusyButton(el, fn)`/`fetchWithTimeout(url, options,
      timeoutMs=12000)` added to `ui.js`. `withBusyButton` disables the
      element for `fn`'s duration and, only for actual `<button>`s,
      swaps in/restores a spinner (checkboxes/selects just get
      `.disabled` — a spinner would clobber a checkbox's own visual
      state or blow away a select's options).

      Deduped the 3 existing copies onto it: `actions.js`'s
      `.btn-universal` handler, `control.js`'s `btn-check-target`,
      `guider-panel.js`'s `postGuider`. Applied to every confirmed-
      unprotected handler from the doc's list: Expose (`control.js`,
      one `withBusyButton` per station button via `e.currentTarget`),
      Loop toggle (`loop-toggle.js`'s `change` handler — checkbox
      variant, `refresh()`'s own 5s poll left alone per the doc's
      "idempotent polling, harmless to overlap" note), stage relative
      (`control.js` — went a bit further than a plain disable: the +
      and - buttons now also disable *each other* while either is
      in-flight, since the doc specifically flagged this as a
      read-then-write race between the two, not just a missing
      disable), `select.select-universal` (`actions.js`), webcam
      move/preset (`webcam.js`, both buttons), sequencer
      save/delete/refresh (`sequencer.js` — `saveOB`/`deleteOB`/
      `refreshBrowser` all take an optional trigger button now;
      `withBusyButton(undefined, fn)` degrades to a plain call, so the
      init-time and post-success internal `refreshBrowser()` calls
      that have no associated button still work unchanged; `deleteOB`
      picked up a missing try/catch as a direct consequence of being
      wrapped, it had none before — matches its siblings' pattern).

      **One deliberate exception**: `control.js`'s `postExpose` uses
      `withBusyButton` (Expose should stay disabled/spinning for a
      real exposure's whole duration) but plain `fetch()`, not
      `fetchWithTimeout` — checked `POST /api/sequencer/run` server-side
      (`noctua/sequencer.py`'s `execute()`) and it blocks synchronously
      until every template in the OB finishes, which for a real
      exposure can be minutes; the ~12-15s default would abort a
      legitimate long exposure. Checked the other endpoints touched
      here don't have this shape before applying the default:
      `/api/{cam}/loop` POST/DELETE and `/api/guider/` POST both just
      spawn a background thread/task and return immediately
      (confirmed by reading `looping`'s setter and `guider.py`'s
      `start()`), `/api/stage/position` PUT just issues a move command
      without waiting for the motor, so the default timeout is safe
      for all of those.

      Verified with `node --check` on every touched JS file.
- [ ] Extract shared `applyTransform()` helper (dedupe
      `status-stream.js`/`status-view.js`).
- [ ] Unify the 4 error-toast-extraction idioms into one helper; fix
+75 −79
Original line number Diff line number Diff line
// actions.js
// Global event handler for interactive control elements using event delegation.

import { showToast } from './ui.js';
import { showToast, withBusyButton, fetchWithTimeout } from './ui.js';

document.addEventListener('DOMContentLoaded', () => {
    // Catch click events globally on body to avoid duplicate handlers per page
@@ -15,11 +15,7 @@ document.addEventListener('DOMContentLoaded', () => {
        const endpoint = button.dataset.url;
        const safeId = button.dataset.safeId;

        // Temporarily disable the button to prevent multiple submissions
        const originalContent = button.innerHTML;
        button.disabled = true;
        button.innerHTML = '<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true"></span>';

        await withBusyButton(button, async () => {
            try {
                let payload = null;

@@ -36,7 +32,7 @@ document.addEventListener('DOMContentLoaded', () => {
                    });
                    payload = values.length === 1 ? values[0] : values;
                } else if (button.dataset.payload !== undefined) {
                // RISOLUZIONE BUG: Supporto per i payload complessi (usato da Sequencer e Expose)
                    // Complex payload support (used by Sequencer and Expose)
                    try {
                        payload = JSON.parse(button.dataset.payload);
                    } catch {
@@ -61,7 +57,7 @@ document.addEventListener('DOMContentLoaded', () => {
                    finalUrl += '?force=true';
                }

            const response = await fetch(finalUrl, {
                const response = await fetchWithTimeout(finalUrl, {
                    method: method,
                    headers: {
                        'Content-Type': 'application/json'
@@ -86,11 +82,9 @@ document.addEventListener('DOMContentLoaded', () => {
            } catch (err) {
                console.error('Action failed:', err);
                showToast(`Error: ${err.message}`, 'danger');
        } finally {
            button.disabled = false;
            button.innerHTML = originalContent;
            }
        });
    });

    // select-universal — fires on change, mirrors btn-universal conventions:
    //   data-url      endpoint (prepended with /api)
@@ -124,8 +118,9 @@ document.addEventListener('DOMContentLoaded', () => {
            payload = sel.value;
        }

        await withBusyButton(sel, async () => {
            try {
            const response = await fetch(url, {
                const response = await fetchWithTimeout(url, {
                    method,
                    headers: { 'Content-Type': 'application/json' },
                    body: payload !== null ? JSON.stringify(payload) : null,
@@ -138,6 +133,7 @@ document.addEventListener('DOMContentLoaded', () => {
                showToast(`Error: ${err.message}`, 'danger');
            }
        });
    });

    // Handle individual widget raw toggles locally
    document.body.addEventListener('change', (event) => {
+70 −54
Original line number Diff line number Diff line
// control.js — Control page: mode switching, stage movement, expose dispatch.

import { showToast, setInputState } from './ui.js';
import { showToast, setInputState, withBusyButton, fetchWithTimeout } from './ui.js';
import { wireLoopToggle } from './loop-toggle.js';

// Map station → { panel id suffix, FITS viewer combo, sequencer template,
@@ -95,11 +95,9 @@ document.addEventListener('DOMContentLoaded', () => {
    btnCheck?.addEventListener('click', async () => {
        const target = inputRadec?.value?.trim();
        if (!target) return;
        const origHtml    = btnCheck.innerHTML;
        btnCheck.disabled = true;
        btnCheck.innerHTML = '<span class="spinner-border spinner-border-sm" aria-hidden="true"></span>';
        await withBusyButton(btnCheck, async () => {
            try {
            const res  = await fetch('/api/telescope/coordinates/resolve', {
                const res  = await fetchWithTimeout('/api/telescope/coordinates/resolve', {
                    method:  'POST',
                    headers: { 'Content-Type': 'application/json' },
                    body:    JSON.stringify(target),
@@ -113,11 +111,9 @@ document.addEventListener('DOMContentLoaded', () => {
                }
            } catch {
                setInputState(inputRadec, 'invalid');
        } finally {
            btnCheck.disabled  = false;
            btnCheck.innerHTML = origHtml;
            }
        });
    });

    inputRadec?.addEventListener('input', () => setInputState(inputRadec, 'reset'));

@@ -137,15 +133,24 @@ document.addEventListener('DOMContentLoaded', () => {
    // Stage relative movement
    // -----------------------------------------------------------------------
    const inputRel = document.getElementById('stage-rel-val');

    async function moveStageRelative(direction) {
    const btnStageRelPlus  = document.getElementById('btn-stage-rel-plus');
    const btnStageRelMinus = document.getElementById('btn-stage-rel-minus');

    // Disabling only the clicked button isn't enough here: this is a
    // read-then-write (GET position, PUT computed from it), so the +
    // and - buttons must also lock each other out, not just themselves,
    // or a quick +/- pair could race on the same stale read.
    async function moveStageRelative(btn, otherBtn, direction) {
        const step  = parseFloat(inputRel?.value) || 0;
        const delta = direction * step;
        if (otherBtn) otherBtn.disabled = true;
        try {
            await withBusyButton(btn, async () => {
                try {
            const res     = await fetch('/api/stage/position');
                    const res     = await fetchWithTimeout('/api/stage/position');
                    const data    = await res.json();
                    const current = data.response || 0;
            await fetch('/api/stage/position', {
                    await fetchWithTimeout('/api/stage/position', {
                        method:  'PUT',
                        headers: { 'Content-Type': 'application/json' },
                        body:    JSON.stringify(current + delta),
@@ -153,10 +158,14 @@ document.addEventListener('DOMContentLoaded', () => {
                } catch {
                    showToast('Stage relative move failed', 'danger');
                }
            });
        } finally {
            if (otherBtn) otherBtn.disabled = false;
        }
    }

    document.getElementById('btn-stage-rel-plus') ?.addEventListener('click', () => moveStageRelative(+1));
    document.getElementById('btn-stage-rel-minus')?.addEventListener('click', () => moveStageRelative(-1));
    btnStageRelPlus ?.addEventListener('click', () => moveStageRelative(btnStageRelPlus,  btnStageRelMinus, +1));
    btnStageRelMinus?.addEventListener('click', () => moveStageRelative(btnStageRelMinus, btnStageRelPlus,  -1));

    // -----------------------------------------------------------------------
    // EXPOSE helpers
@@ -176,8 +185,14 @@ document.addEventListener('DOMContentLoaded', () => {
        return params;
    }

    async function postExpose(template, camera, params) {
    async function postExpose(btn, template, camera, params) {
        await withBusyButton(btn, async () => {
            try {
                // No fetchWithTimeout here: /api/sequencer/run blocks server-side
                // until the whole OB finishes executing (sequencer.py's execute()
                // runs every template synchronously), which for a real exposure
                // can legitimately take far longer than the ~12-15s timeout used
                // everywhere else — aborting this one would kill real exposures.
                const res  = await fetch('/api/sequencer/run', {
                    method:  'POST',
                    headers: { 'Content-Type': 'application/json' },
@@ -191,21 +206,22 @@ document.addEventListener('DOMContentLoaded', () => {
            } catch {
                showToast('Sequencer POST failed', 'danger');
            }
        });
    }

    document.getElementById('btn-imaging-expose')?.addEventListener('click', () => {
    document.getElementById('btn-imaging-expose')?.addEventListener('click', (e) => {
        const form = document.getElementById('form-imaging');
        if (form) postExpose(MODES.station1.template, MODES.station1.camera, readForm(form));
        if (form) postExpose(e.currentTarget, MODES.station1.template, MODES.station1.camera, readForm(form));
    });

    document.getElementById('btn-spectro-expose')?.addEventListener('click', () => {
    document.getElementById('btn-spectro-expose')?.addEventListener('click', (e) => {
        const form = document.getElementById('form-spectro');
        if (form) postExpose(MODES.station2.template, MODES.station2.camera, readForm(form));
        if (form) postExpose(e.currentTarget, MODES.station2.template, MODES.station2.camera, readForm(form));
    });

    document.getElementById('btn-echelle-expose')?.addEventListener('click', () => {
    document.getElementById('btn-echelle-expose')?.addEventListener('click', (e) => {
        const form = document.getElementById('form-echelle');
        if (form) postExpose(MODES.station3.template, MODES.station3.camera, readForm(form));
        if (form) postExpose(e.currentTarget, MODES.station3.template, MODES.station3.camera, readForm(form));
    });

    // -----------------------------------------------------------------------
+15 −20
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@
// Auto-discovers panel instances from the DOM: any element whose id matches
// "btn-{panelId}-guide-start" triggers initialisation of that panel.

import { showToast }           from './ui.js';
import { showToast, withBusyButton, fetchWithTimeout } from './ui.js';
import { get as getViewer }    from './viewer/viewer-registry.js';
import { resolveValue }        from './ui-core.js';
import { computeTargetCoords } from './guider-target-coords.js';
@@ -134,12 +134,9 @@ function collectParams(panelId) {
 * @param {object} params
 */
async function postGuider(btn, params) {
    const orig    = btn.innerHTML;
    btn.disabled  = true;
    btn.innerHTML = '<span class="spinner-border spinner-border-sm" aria-hidden="true"></span>';

    await withBusyButton(btn, async () => {
        try {
        const res = await fetch('/api/guider/', {
            const res = await fetchWithTimeout('/api/guider/', {
                method:  'POST',
                headers: { 'Content-Type': 'application/json' },
                body:    JSON.stringify(params),
@@ -150,10 +147,8 @@ async function postGuider(btn, params) {
            }
        } catch (err) {
            showToast(`Guider: ${err.message}`, 'danger');
    } finally {
        btn.disabled  = false;
        btn.innerHTML = orig;
        }
    });
}

// --- 3. Pick mode ---
+10 −6
Original line number Diff line number Diff line
@@ -5,6 +5,8 @@
// (teccam) — each camera has exactly one such switch in the page, so no
// cross-widget sync is needed here.

import { withBusyButton, fetchWithTimeout } from './ui.js';

/**
 * @param {HTMLInputElement} el - checkbox input, its .checked IS the loop state.
 * @param {function(): (string|null)} getCamId - resolves the cam_id (e.g. "teccam1").
@@ -76,13 +78,15 @@ export function wireLoopToggle(el, getCamId, getExptime, getBinning, getGain, on
            if (gain != null) body.gain = gain;
            options.body = JSON.stringify(body);
        }
        await withBusyButton(el, async () => {
            try {
            const res = await fetch(`/api/${camId}/loop`, options);
                const res = await fetchWithTimeout(`/api/${camId}/loop`, options);
                setChecked(res.ok ? wantOn : !wantOn);
            } catch {
                setChecked(!wantOn);
            }
        });
    });

    refresh();
    return { refresh };
Loading