bigskysoftware / htmx

</> htmx - high power tools for HTML
https://htmx.org
Other
37.84k stars 1.28k forks source link

htmx:afterRequest not fired if a string is sent from the server on 5xx error #2837

Open DreamwareDevelopment opened 1 month ago

DreamwareDevelopment commented 1 month ago

I thought maybe this was a caveat of AJAX expecting xml but when I send a JSON object the event fires as expected. Super not urgent issue for me either, was just scratching my head why my buttons weren't getting re-enabled. Easy enough to return JSON and get the error that way.

DreamwareDevelopment commented 1 month ago

Actually, nevermind. JSON doesn't work either when there is some delay added to the server response.

Edit: Just checked, no response sent from the server besides the 500 code is still not triggering the htmx:responseError or htmx:afterRequest events when there is delay introduced via the server. Am I doing something wrong?

document.body.addEventListener('htmx:beforeRequest', function(event) {
    // Find all buttons that have a loading span inside them
    any('button')?.forEach(button => {
      if (button.querySelector('.loading-span')) {
        console.log(`Disabling button from beforeRequest`);
        console.log(button);
        button.disabled = true;
      }
    });
  });

  document.body.addEventListener('htmx:responseError', function(event) {
    console.log(`htmx:responseError`);
    // Re-enable all buttons that have a loading span inside them
    any('button')?.forEach(button => {
      if (button.querySelector('.loading-span')) {
        console.log(`Re-enabling button from responseError`);
        console.log(button);
        button.disabled = false;
      }
    });
  });

  document.body.addEventListener('htmx:afterRequest', function(event) {
    console.log(`htmx:afterRequest`);
    // Re-enable all buttons that have a loading span inside them
    any('button')?.forEach(button => {
      if (button.querySelector('.loading-span')) {
        console.log(`Re-enabling button from afterRequest`);
        console.log(button);
        button.disabled = false;
      }
    });
  });
<form 
        id="bulkUploadForm" 
        class="w-full flex flex-col gap-4" 
        hx-encoding="multipart/form-data"
        hx-post="/s/monitoring/upload"
        hx-target="body"
        hx-swap="outerHTML"
        hx-indicator=".loading-span"
      >
Telroshan commented 1 month ago

Hey, could you provide a reproducible example (either a JSFiddle/Codepen, or a repo)? A quick test on this JSFiddle seems to behave as expected, am I missing something? Note that this code gets 404 errors and not 5xx, though 4xx and 5xx error code should get the same treatment, as they are both part of the same responseHandling default config. https://github.com/bigskysoftware/htmx/blob/bd2dc6564d3b6fe91cf94f8efb39ac8b776ea48b/src/htmx.js#L264-L268

DreamwareDevelopment commented 1 month ago

Dang it. My minimal example works fine. I'll have to keep digging into my code to find out where things are going wrong. I'll close this for now.

DreamwareDevelopment commented 1 month ago

Reopening, I reproduced the issue in my minimal example and now I understand what's going wrong. Essentially, I have a form in a modal. That modal has it's own onclick handler on the submit button that starts an animation to fade out the modal. At this same time, the post request is made. However, there is a listener on animationend that removes the modal from the DOM. So, when the request comes back later, the event is processed by HTMX, however because the original node that issued the request is no longer in the DOM, a JS exception is thrown and JS stops working. Could this be handled more gracefully?

DreamwareDevelopment commented 1 month ago

I made a workaround (not in the example repo) that waits to remove the element that triggered the request until there is no htmx-request class present. That unblocks me, but there may still be some improvements that could be made here, so leaving this open.

Telroshan commented 1 month ago

On a UX aspect, I would suggest waiting for the request to complete before dismissing the modal anyway; what if, for any reason, the request fails, wouldn't you want your modal to stay open with potentially an error message instead of having it dismissed, giving the user the illusion that everything went well even though it didn't?