dotnet / aspire

An opinionated, cloud ready stack for building observable, production ready, distributed applications in .NET
https://learn.microsoft.com/dotnet/aspire
MIT License
3.61k stars 401 forks source link

Console logs: Link detection does not properly parse full URLs #1105

Open onionhammer opened 9 months ago

onionhammer commented 9 months ago

image

When I hover over the URL, it excludes the port, etc.

This may have to do with colored port numbers vite spits out.

tlmii commented 9 months ago

Good call on the colorization. I bet that's the cause here. Out of curiosity, does the URL detection work for VITE output in plain windows terminal? We more or less match what they do for detection, though we might be doing it at a different stage of the pipeline that has different ultimate effect...

onionhammer commented 9 months ago

does the URL detection work for VITE output in plain windows terminal

Yes, it works in VS Code terminal as well

tlmii commented 9 months ago

Interesting. We'll have to see if we can re-layer that logic (url detection and ANSI-to-CSS are basically two of several steps in the log processing). Good find! We'll take a look.

MatsM16 commented 6 months ago

I am unable to recreate this issue in the latest main.
I have tried looking at the logs with urls in them and I have tried the following tests:

  [Theory]
  [InlineData("No urls", "No urls")]
  [InlineData("Named https url: https://some-page.com/", "Named https url: <a target=\"_blank\" href=\"https://some-page.com/\">https://some-page.com/</a>")]
  [InlineData("Named http url: http://some-page.com/", "Named http url: <a target=\"_blank\" href=\"http://some-page.com/\">http://some-page.com/</a>")]
  [InlineData("IP https url: https://0.0.0.0/", "IP https url: <a target=\"_blank\" href=\"https://0.0.0.0/\">https://0.0.0.0/</a>")]
  [InlineData("IP https url: http://0.0.0.0/", "IP https url: <a target=\"_blank\" href=\"http://0.0.0.0/\">http://0.0.0.0/</a>")]
  [InlineData("Named https url with port: https://some-page.com:5173/", "Named https url with port: <a target=\"_blank\" href=\"https://some-page.com:5173/\">https://some-page.com:5173/</a>")]
  [InlineData("Named http url with port: http://some-page.com:5173/", "Named http url with port: <a target=\"_blank\" href=\"http://some-page.com:5173/\">http://some-page.com:5173/</a>")]
  [InlineData("IP https url with port: https://0.0.0.0:5173/", "IP https url with port: <a target=\"_blank\" href=\"https://0.0.0.0:5173/\">https://0.0.0.0:5173/</a>")]
  [InlineData("IP http url with port: http://0.0.0.0:5173/", "IP http url with port: <a target=\"_blank\" href=\"http://0.0.0.0:5173/\">http://0.0.0.0:5173/</a>")]
  [InlineData("Named https url: https://some-page.com", "Named https url: <a target=\"_blank\" href=\"https://some-page.com\">https://some-page.com</a>")]
  [InlineData("Named http url: http://some-page.com", "Named http url: <a target=\"_blank\" href=\"http://some-page.com\">http://some-page.com</a>")]
  [InlineData("IP https url: https://0.0.0.0", "IP https url: <a target=\"_blank\" href=\"https://0.0.0.0\">https://0.0.0.0</a>")]
  [InlineData("IP https url: http://0.0.0.0", "IP https url: <a target=\"_blank\" href=\"http://0.0.0.0\">http://0.0.0.0</a>")]
  [InlineData("Named https url with port: https://some-page.com:5173", "Named https url with port: <a target=\"_blank\" href=\"https://some-page.com:5173\">https://some-page.com:5173</a>")]
  [InlineData("Named http url with port: http://some-page.com:5173", "Named http url with port: <a target=\"_blank\" href=\"http://some-page.com:5173\">http://some-page.com:5173</a>")]
  [InlineData("IP https url with port: https://0.0.0.0:5173", "IP https url with port: <a target=\"_blank\" href=\"https://0.0.0.0:5173\">https://0.0.0.0:5173</a>")]
  [InlineData("IP http url with port: http://0.0.0.0:5173", "IP http url with port: <a target=\"_blank\" href=\"http://0.0.0.0:5173\">http://0.0.0.0:5173</a>")]
  public void CreateLogEntry_LinkDetectionParsesFullUrls(string input, string expectedOutput)
  {
      var logParser = new LogParser(false);
      var logEntry = logParser.CreateLogEntry(input, false);
      var output = logEntry.Content;

      Assert.Equal(expectedOutput, output);
  }

In all tests, LogParser correctly parses the full url and generates a link.
If wanted, I can submit a pr adding this test Aspire.Dashboard.Tests.
In either case, I am convinced the issue has been resolved.

Edit:
The tests tries to validate LogParser as a whole. UrlParser is already well tested.

tlmii commented 6 months ago

I don't think we've done anything to address the issue. The tests above wouldn't catch the problem. The issue (based on the info provided by the OP) is that VITE passes ANSI control codes to colorize the port number (and the colon and forward slash chars). So the text coming across in the logs to generate the URL in the OP's image (just the URL, ignoring the arrow and other text on that line) would be something like http://localhost\x1B[94m:\x1B[96m5173x1B[94m/x1B[0m.

We would need to do something such that the URL parsing can ignore the ANSI codes, or some other way to orchestrate both changes.

MatsM16 commented 6 months ago

What would be the preferred way to parse http://localhost\x1B[94m:\x1B[96m5173x1B[94m/x1B[0m:

  1. <a target="_blank" href="http://localhost:5173/">http://localhost:5173/</a>
    Completely ignoring ANSI sequences.
  2. <a target="_blank" href="http://localhost:5173/">http://localhost\x1B[94m:\x1B[96m5173x1B[94m/x1B[0m</a>
    Only removing ANSI sequences for the actual href.

Also, would it be ok if I made AnsiParser.IsControlSequence and AnsiParser.EscaoeChar public to reuse them in UrlParser?

Edit: I realize UrlParser will receive the ANSI sequences as <span>...</span> due to the AnsiParser running first.

MatsM16 commented 6 months ago

@tlmii I have made a pr changing UrlParser to remove xml tags when parsing an URL.
I would love some input.

JamesNK commented 5 months ago

I think a true fix here is relatively large.

The stress playground project I added recently includes some examples of URLs mixed with colors. If the ApiService is started directly then you can see terminals detects the URLs mixed with colors: url-color-encoding

I think the log parser needs to be completely redesigned to:

  1. Parse text to tree of tokens
  2. Have a way to get the raw text from the tokens
  3. Run URL regex on raw text (and do other log parsing)
  4. Find tokens based on index of raw text (start and end of urls, time stamps, log levels, etc)
  5. Modify tree to add URL token and then nest/split text and color tokens to fit inside
  6. Render tokens to HTML

I think there is too much here to recommend the issue for a contributor.

kvenkatrajan commented 5 months ago

@JamesNK - if this is a bigger change from a risk perspective, we would like to consider it post GA -please let me know if you have any concerns.

davidfowl commented 5 months ago

Punting this to 8.1