Show server response as error message in put saver #6589
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Second commit is kind of unrelated, so let me know if you prefer it removed from this PR. |
|
Thanks @simonbaird looks OK to me, but it would indeed be helpful to test it with WebDAV backends. I think @pmario and @saqimtiaz have done some WebDAV experiments in the last year or two? |
|
I use WebDAV pretty heavily and can do some testing this week. I don't foresee any issues though. |
|
I'm using WebDav for some of my wikis and IIS as backend. I'll test it and report back |
simonbaird
force-pushed
the
put-saver-server-messages
branch
2 times, most recently
from
57c6b79 to
5457d85
Compare
I'd like to use this on Tiddlyhost so users can get more informative error messages if the put save fails for whatever reason. This would make the put saver a viable replacement for the legacy upload saver, which is what Tiddlyhost uses currently. I'm not sure what standard WebDAV servers do, but I would guess they don't provide any response body for put requests, and hence this patch would have no impact for a standard WebDAV server. (That said, it would be a good idea to test it to make sure there aren't any unexpected regressions for WebDAV or other put saver compatible services.)
There's no need to extract it from the error string created inside tw.utils.httpRequest if we can get it directly from the xhr object.
There are two related changes here: 1. Add a 'Save starting' notification for the put saver, similar to the upload saver. Not sure if it was intentionally omitted for the put saver, but it seems reasonable to have the two be consistent. 2. Send the 'Save starting' notifications in both upload and put save right before the actual request is sent. While testing I noticed that the save might have failed before the "Save starting" notification appeared which doesn't seem useful.
simonbaird
force-pushed
the
put-saver-server-messages
branch
from
5457d85 to
348de46
Compare
|
I added another commit to add a "Save started" notification for put saving. See commit message for details. |
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 4, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 4, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 5, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
I can confirm that a response to a PUT request from a WebDAV server has no response body for a successful request. A bit trickier to recreate error conditions, but will have another look later in the week. |
|
Thanks everyone I think that we can merge this now, but obviously further testing is welcome. |
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 5, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 6, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
|
Thanks all! If you want to see this in action on Tiddlyhost, follow the steps here. |
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 6, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 7, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to tiddlyhost/tiddlyhost-com
that referenced
this pull request
Apr 10, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
simonbaird
added a commit
to simonbaird/tiddlyhost-app
that referenced
this pull request
Apr 27, 2022
TiddlyWiki's upload.js saver is old and deprecated so let's see if we can switch to using the put saver. Includes: - A bogus WebDAV header to convince TiddlyWiki that the put saver is usable - A new controller method and route for the "put" saves - Clear the url from $:/UploadURL when serving a site to make sure TW doesn't try to use the upload saver There's a related PR for TiddlyWiki that exposes the error messages to the user, see TiddlyWiki/TiddlyWiki5#6589 Issue: #148
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'd like to use this on Tiddlyhost so users can get more informative error messages if the put save fails for whatever reason.
This would make the put saver a viable replacement for the legacy upload saver, which is what Tiddlyhost uses currently.
I'm not sure what standard WebDAV servers do, but I would guess they don't provide any response body for put requests, and hence this patch would have no impact for a standard WebDAV server.
(That said, it would be a good idea to test it to make sure there aren't any unexpected regressions for WebDAV or other put saver compatible services.)
Also includes two related small tweaks. See commit messages for details.