search

HangfireIO / Hangfire

An easy way to perform background job processing in .NET and .NET Core applications. No Windows Service or separate process required
https://www.hangfire.io
Other
9.43k stars 1.71k forks source link

Improve display of job result and properties (followup) #2396

Open 0xced opened 7 months ago

0xced commented 7 months ago

This is a followup to #1509 which was closed on the ground that the new UseJobDetailsRenderer method could be used.

While technically possible to achieve (see https://github.com/0xced/Hangfire.TidyDashboard) it's a lot of work: reimplement the Hangfire.Dashboard.JobHistoryRenderer.SucceededRenderer method in order to prettify the JSON and the newlines and wrap a JobStorageMonitor in order to prettify the parameters.

It think this makes much more sense to have this prettifying of strings built-in to the Hangfire dashboard.

[!NOTE]
I have also fixed a bug compared to the original pull request where non-string job parameters would be double html escaped.

Original description

It's quite common that job properties and/or result are strings. Since everything is serialized as JSON in the database, a string becomes quoted and has escaped characters. For displaying in the dashboard, it's nicer without the quotes and escaped characters.

Before this pull request:

HangFire-ugly

After this pull request:

HangFire-pretty
odinserj commented 4 months ago

Hi @0xced! I'd still like to stand with the solution posted in this comment – https://github.com/HangfireIO/Hangfire/pull/1509#issuecomment-1515753447. The main reason for this is to have both parameters and return value formatted as JSON string by default to avoid confusion and present the value as is. Otherwise it might be not clear which type is behind the value, especially for numbers encoded as strings.

Currently they will be displayed as "12345", and integer-based values as 12345, proving a clear information about the actual type (string vs int). If we change that, it would be totally unclear what is the actual type behind the value.

I don't like the solution to special-case strings and forgetting other types, because it's not consistent. General solution to this problem would be possible by converting all such values from JSON to their actual type first (string/int/CustomDto), and then calling their ToString method. But not all the types have this method implemented, especially different custom DTOs, and sometimes it's impossible to identify the expected type -- for example, job parameters are converted dynamically, and no information about their actual type is stored, conversion is performed in code.