open Header.Writer class and create GetBytesLength method #674
Conversation
|
@SiuTung08 should we provide a convenience method on the header class so you can call it like |
Make sense! Just pushed the commit for this. |
|
@mtmk looks like the test jobs for Linux (latest) failed. Looking at the actual results though it seems that all tests have passed. Can we retrigger it? |
src/NATS.Client.Core/NatsHeaders.cs
Outdated
| /// </summary> | ||
| /// <param name="headerWriter">an instance of headerWriter</param> | ||
| /// <returns>bytes length of th header</returns> | ||
| public long GetBytesLength(HeaderWriter headerWriter) |
There was a problem hiding this comment.
just thinking about it from the application developer standpoint: is there a way we can make this taking an optional encoding rather than the writer? maybe in that case we won't even need to expose a writer? e.g. have an internal static GrtBytesLength(Encoding) method on the writer where we can reuse here?
public long GetBytesLength(Encoding? encoding = null)
{
// e.g. if null set to utf-8what do you think?
There was a problem hiding this comment.
@mtmk I agree, I will make some changes.
| throw new NatsException( | ||
| $"Invalid header key '{kv.Key}': contains colon, space, or other non-printable ASCII characters"); | ||
| } | ||
| throw new NatsException($"Invalid header key '{kv.Key}': contains colon, space, or other non-printable ASCII characters"); |
There was a problem hiding this comment.
- @mtmk correct me if I'm off here, do we prefer including braces?
- If possible, this would be a good opportunity to wrap this
throwinto a helper method, e.x.
if (!ValidateKey(keySpan.Slice(0, keyLength)))
{
NatsException.Throw($"Invalid header key '{kv.Key}': contains colon, space, or other non-printable ASCII characters")
}
public class NatsException : Exception
{
//etc
[MethodImpl(MethodImplOptions.NoInlining)]
public static void Throw(string message)
{
throw new NatsException(message);
}
//etc
}
(There may be a throwhelper already floating around, if so prefer that)
Main advantage of such is we can get some better inlining/opts and avoid certain callstack checks on happy path....
There was a problem hiding this comment.
ouch just merged it. I shall open a new PR
EDIT
* Add OnSocketAvailableAsync Hook (#647) * Refactor exception handling in NatsException (#677) * Fixed an exception which happens when PutAsync is used more than once and activity logging is enabled in main project (#675) * open Header.Writer class and create GetBytesLength method (#674) * Fix date time serialization to use ISO8601 (#664)
* Add OnSocketAvailableAsync Hook (#647) * Refactor exception handling in NatsException (#677) * Fixed an exception which happens when PutAsync is used more than once and activity logging is enabled in main project (#675) * open Header.Writer class and create GetBytesLength method (#674) * Fix date time serialization to use ISO8601 (#664)
Open Header.Writer class and create GetBytesLength method for calculating NatsHeaders size (length in bytes) up front to call Publish for large Nats message which may exceeds payload size against server's maximum payload size.