Skip to content

Conversation

@R0Wi
Copy link
Member

@R0Wi R0Wi commented Feb 12, 2025

  • Check local socket by assuming it starts with a slash
  • Improve error logging for ping and deploy

Addresses #498

Log messages now look like this when doint a ping via UI (if either the docker socket is not writable or the file path doesn't exist)

socket doesn't exist:
socket-does-not-exist

socket is not writable:
socket-not-writable

Also the frontend dialog for the deployment now shows

deployment error for local socket:
deploy-result-err

deployment error for nextcloud DSP:
deploy-result-err-dsp

Unfortunately I didn't find a way to get rid of the slightly misleading error message that - in case the socket is not usable - curl will always complain about not being able to connect tho http://localhost/, even though it's not using any http under the hood but trying to access the UNIX socket directly. When using curl in socket mode ( CURLOPT_UNIX_SOCKET_PATH), it will still need some http-address for the actual GET call. See also curl/curl#1338

* Check local socket by assuming it starts with a slash
* Improve error logging for ping and deploy

Signed-off-by: Robin Windey <[email protected]>
@R0Wi R0Wi requested a review from oleksandr-nc February 12, 2025 20:03
@oleksandr-nc
Copy link
Contributor

@kyteinsky or @andrey18106 - second review?

note: we will backport this to the NC31/NC30

Copy link
Contributor

@andrey18106 andrey18106 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@R0Wi R0Wi enabled auto-merge February 13, 2025 16:45
@R0Wi R0Wi merged commit 12ee1b7 into main Feb 13, 2025
32 checks passed
@R0Wi R0Wi deleted the fix#498 branch February 13, 2025 16:46
@R0Wi
Copy link
Member Author

R0Wi commented Feb 13, 2025

/backport to stable31

@R0Wi
Copy link
Member Author

R0Wi commented Feb 13, 2025

/backport to stable30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants