Pjax visits URL after server returned error 500 on form submission

misog commented 2 years ago

Describe the bug

Hi, when a form is submitted and server return code 500 then Pjax tries to visit the form URL with GET again. This can result in 405 Method not allowed. See:

Screenshot from 2022-07-30 15-41-12

To reproduce

Submit a form with method POST and URL with Pjax
Return 500 from server
See that Pjax visits the URL with GET method

Expected behavior

Response is displayed and no second request is made.

Environment

- Browser: Firefox ESR 91.12.0
- Pjax: 2.4.0

Anything else?

No response

PaperStrike commented 2 years ago

This is most likely caused by that Pjax falls back to normal page load (via window.location.assign, which is a GET) when navigating to a page (not limited to form submissions) where the given selectors does not select the same amount of DOM elements. That is, your 500 response wasn't attached with a structure-matching document body.

This does not meet the standard/default behavior that it should navigate to the response document without sending a new request regardless of whether the response body is empty and whether the structure matches. However, the currently available Web APIs do not allow Pjax to achieve the standard behavior. There's no API to "terminate" all page scripts (documents with mismatched structure are considered incompatible with Pjax documents, so a full/standard/browser navigation is required), and no API to show the browser default 500 page. The best solution for Pjax I can think of is a window.location.assign, if you have a better and feasible idea you can post it here. Thank you.

Possible solutions for you:

Configure your server to use a custom 500 page that's compatible with the form page structure when sending 500 responses.
Disable Pjax on the form / Let the browser handle the form submission. (defaultTrigger.exclude)
Handle the form submission yourself. (listen to the related form events, call event.preventDefault(), send the request and handle the response yourself. Pjax will automatically ignore events that were default-prevented.)

misog commented 2 years ago

Thank you for possible solutions. Will you create Pjax config option to modify the current behavior?

misog commented 2 years ago

Hi, I think that Pjax should not create second request. It should display html content if available, for example with document.write because there can be html error descripttion with error 500.

Pjax could be also configured to handle different mime types if no html is available.

PaperStrike commented 1 year ago

Pjax can be seperated into two parts, the core and the "trigger". The core handles page navigations, the trigger triggers the navigations.

As in defaultTrigger docs, you can use load on your demand. As in load, window.location.assign is used for almost every kind of error, it is a documented behavior. As right below load, weakLoad, you can use weakLoad to handle all the errors on your own, with no automatic window.location.assign.

Q Why the assign is used for almost every kind of error? Q Why the assign is used for POST? Q Why use an assign to hide the real HTTP status code?

As in the previous comment, currently there's no suitable API to achieve the desired native navigation behavior.

There is really no good choice for Pjax. In a native navigation, the browser unloads the current page and loads the new page. Pjax is used to avoid the unload of structure-matching pages and to simulate the load of such pages. The available Web APIs can only simulates the load, not the unload.

Q Why not ignore the unload and just replace all the elements? Q Isn't document.write suitable for page unload?

These are all more prone to errors. Most scripts are designed to work with normal (not html/head/body-replaced) pages, they may rely on some particular elements, some particular variables, etc. Scripts in Pjax sites are designed to work with structure-matching pages.

If you insist on using document.write, be sure you are good with [the HTML spec warnings](https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document.write()). And it doesn't seem to clean global variables, JS event loop, workers, message ports, import map, ..., which may lead to much more and much uncommon errors.

The best way to handle the errors is to handle on your own. Different sites have different preferences. Some may prefer to show a dialog on errors, some may prefer to show a text next to the form, while some may prefer to redirect to a different page. Different servers may have different error message formats, too. Refer to the last two points of my last comment for some basic usages.

Note that load and weakLoad can accept Request objects.

As of v2.4.0, load is a simple wrapper of weakLoad. So if you want to handle the errors on your own while still using the default trigger, technically you can simply set it to your custom function. The default trigger will use the load to trigger Pjax. For example,

// UNOFFICIAL usage.
var pjax = new Pjax({
  // ...
});
pjax.load = (req) => pjax
  .weakLoad(req)
  .catch((e) => {
    // ... your own error handling
    // `e.detail` has more details like the request, response, document...
  });

The default trigger is not fully tested. It's more encouraged to write your own trigger for your specific case to catch and handle the errors yourself. (listen to the related link/form events yourself, call event.preventDefault() yourself, generate the request yourself, send and handle the response yourself or via Pjax weakLoad.)

PaperStrike / Pjax