Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

too long dag description causes scoket error #395

Closed
altmvogi opened this issue Mar 8, 2023 · 4 comments · Fixed by #593
Closed

too long dag description causes scoket error #395

altmvogi opened this issue Mar 8, 2023 · 4 comments · Fixed by #593
Labels
bug Something isn't working

Comments

@altmvogi
Copy link

altmvogi commented Mar 8, 2023

when teh description of one dag is too long, the dag is not start and the error in std_out looks like this.

sample:
"Rapid7_insightVM_rapid7_insightvm_copy_data_to_tenable_tables"
error:
2023/03/08 03:37:39 failed to start socket server listen unix /tmp/@dagu-Rapid7_insightVM_rapid7_insightvm_copy_data_to_tenable_tables-d8e3ed564c22705f7e2958adbeeb3eb5.sock: bind: invalid argument

locks like for a socket server name there is a length restriction. i would try to figure out the maximum length and do not allow input on the WEB UI larger than the maximum field length.

regards
Michael

@yohamta yohamta added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Dec 9, 2023
@User-26
Copy link

User-26 commented Jan 28, 2024

Same thing. I've set long and descriptive name for a DAG and it stopped working. It looks like a path to a Unix socket cannot be more than 108 bytes long.

@yohamta
Copy link
Collaborator

yohamta commented Jan 28, 2024

Thanks a lot for reporting the issue. I think we can fix this logic to avoid the max length error.
https://github.com/dagu-dev/dagu/blob/69515e19a7ae612fe66ff935862b7af1994092a8/internal/dag/dag.go#L74-L81

@yohamta yohamta added issue confirmed and removed help wanted Extra attention is needed labels May 11, 2024
@zph
Copy link
Contributor

zph commented Jun 26, 2024

I hit this today with a verbose description of the task too :).

@yohamta, I'm happy to contribute to fixing it and want to understand if you have preferences on implementation.

My first approach would be to simplify the socket by making it only use the md5 so it avoids the irregularities that occur if we truncate the name and append an md5.

like so

fmt.Sprintf("@dagu-%x.sock", bs)

But I wonder if you want a fragment of task name in there for human simplicity in debug cases? If so, I recommend keeping the first N-characters of the task name such that it always stays within the limit of socket lengths, eg:

pseudocode

`@dagu-${truncate(taskName)}-MD5.sock`

Such that it's always < 108 bytes.

Do you have a preference between those two implementations? The first seems less brittle but the second seems a little more user friendly during debug.

zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
@zph
Copy link
Contributor

zph commented Jun 26, 2024

Ok, pushed up the @dagu-${truncate(taskName)}-MD5.sock approach for review and added tests.

zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
zph added a commit to zph/dagu that referenced this issue Jun 26, 2024
Ensures that we truncate task names to no more than 60 chars so they
don't exceed the total length limit of 108 that's impose by unix.

Fixes: dagu-org#395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants