8 ms·

> If you "accidentally" implement Write([]byte) (int, error), odds are it'll still be doing something at least semi-sensible if it is accidentally used as an io

by ezrast 4y ago

> If you "accidentally" implement Write([]byte) (int, error), odds are it'll still be doing something at least semi-sensible if it is accidentally used as an io.Writer. Funny, not too long ago I encountered a logging library with a writer that expected each write call to pass in a fully-formed log message, which happened to be in a JSON format. That meant that if you passed the writer to anything that expected a properly byte-oriented stream - like, say, a JSON serialization library - the writing would typically be done in chunks, and each chunk would be sent in isolation to a remote server that was expecting fully-formed JSON, and that server would silently drop the malformed data. It was weeks before I figured out what was going wrong. This would be a great refutation of the article if it had happened in Go, but no, this was Rust, and the library authors had explicitly marked their type as `impl std::io::Write` without understanding why that wasn't appropriate. I guess the moral is that semi-sensible isn't good enough: the real danger isn't that you end up shooting a physical gun; it's that you shoot your video game gun in a subtly wrong way that takes ages to track down. Failing to compile is loads better.

HelloNurse 4y ago
The problem is that the logging library isn't typed enough: it should require some JSON document object instead of text. If it requires text, it must actually accept text. Silently dropping log messages, without even logging that logging is failing, is an even worse defect since it is also terrible without typing errors, e.g. in the case of clients earnestly trying to pass JSON as text but making some mistake with quotation, escaping, commas etc because it is text. So the logging library is inadequately designed and not "semi-sensible" at all.
- ezrast 4y ago
  While serde is the de facto standard for JSON in Rust, it's not actually in the standard library, so there's no one true "JSON document" type for the logger to accept. Anyway, the point of the JSON serializer taking a writer is so that it can push tokens straight to an IO device (the network, in this case), one by one, without having to hold the entire abstract structure in memory. And even if you had a dedicated "JsonWriter" interface for streaming that validates on the way out, you'd have cases where a string is already known to be valid JSON but to send it you have to pay the cost of an extra validation for no good reason. I love me some strong, static types, but sometimes you gotta let bytes be bytes. The silently dropping bad input, yeah, that part was just bad developer experience on the vendor's part.
  - tialaramex 4y ago
    If they want a whole JSON Document here - which apparently they do - but they don't want to just admit that everybody uses serde, they can provide a Trait like OurJSONLogRecord and then provide the implementation for OurJSONLogRecord on the serde type. You can implement it on whatever internal JSON documents are ready to be logged. This OurJSONLogRecord still surfaces the undocumented assumption, "Oh, we need the entire JSON document, we didn't realise anybody would want to stream data" whereas the Write trait does not.
tialaramex 4y ago
> the library authors had explicitly marked their type as `impl std::io::Write` without understanding why that wasn't appropriate. :( Name the library so people can avoid it? Or did they fix this and drop broken versions?
- ezrast 4y ago
  It's the SDK for Fastly's edge computing service, so not something you're likely to run into without getting paid for it. And no, still has the brokenness, still has the internal comment warning their own engineers not to do it wrong[1]. I'm doubtful that my support ticket about the issue ever made it to anyone who actually knows Rust. (edit to add: the feature was in beta at the time and we were early adopters, so Support not knowing how to field issues about it was sorta understandable. Their engineers seemed cool when I had the chance to talk to them) [1] https://docs.rs/fastly/0.8.5/src/fastly/log.rs.html#171 https://docs.rs/fastly/0.8.5/src/fastly/log.rs.html#171
  - tialaramex 4y ago
    That Fastly documentation doesn't read as though it cares about JSON. Are you sure the JSON requirement isn't in some other system?
    - ezrast 4y ago
      The logging utilities are just a frontend to a bunch of supported logging/tracing providers, many but not all of which accept plain text. So yes, it was technically a separate system (Honeycomb) doing the rejection, but one that was blessed by the vendor. And the only debugging facility I had available, which was duplicating the payload to stdout and tailing that, masked the issue because stdout is an actual stream. And to be clear, the system would be broken for the plaintext providers too, just more obviously broken as you'd presumably have a bunch of JSON tokens (or segments of a format string or whatever) show up as individual log lines.
actionfromafar 4y ago
I find it so interesting that there is such a huge chasm between "what we know can be done" and "what we do in practice in industry". How can it be that in so many programming languages, adhering to a contract means nothing but "these methods have similar calling signatures" and "I pinky swear I implement the semantics of std::io::Write " ? Haven't the Ocaml people entered the next metaphysical level of understanding? When they speak of "types" they don't mean "uint32_t" or a class.
- Zababa 4y ago
  What does this have to do with OCaml?
  - actionfromafar 4y ago
    They seem to operate at much higher level of abstraction. They mean something more profound when they speak of "types", which I don't understand. Also Ada seems interesting, with more spelled out contracts.
    - Zababa 4y ago
      That depends on the types. You can have something as basic as "int", something a bit more complex like arrays, or something even more complex like GADTs https://v2.ocaml.org/manual/gadts.html https://v2.ocaml.org/manual/gadts.html

by ezrast 4y ago

HelloNurse 4y ago

The problem is that the logging library isn't typed enough: it should require some JSON document object instead of text. If it requires text, it must actually accept text. Silently dropping log messages, without even logging that logging is failing, is an even worse defect since it is also terrible without typing errors, e.g. in the case of clients earnestly trying to pass JSON as text but making some mistake with quotation, escaping, commas etc because it is text. So the logging library is inadequately designed and not "semi-sensible" at all.

ezrast 4y ago
While serde is the de facto standard for JSON in Rust, it's not actually in the standard library, so there's no one true "JSON document" type for the logger to accept. Anyway, the point of the JSON serializer taking a writer is so that it can push tokens straight to an IO device (the network, in this case), one by one, without having to hold the entire abstract structure in memory. And even if you had a dedicated "JsonWriter" interface for streaming that validates on the way out, you'd have cases where a string is already known to be valid JSON but to send it you have to pay the cost of an extra validation for no good reason. I love me some strong, static types, but sometimes you gotta let bytes be bytes. The silently dropping bad input, yeah, that part was just bad developer experience on the vendor's part.
- tialaramex 4y ago
  If they want a whole JSON Document here - which apparently they do - but they don't want to just admit that everybody uses serde, they can provide a Trait like OurJSONLogRecord and then provide the implementation for OurJSONLogRecord on the serde type. You can implement it on whatever internal JSON documents are ready to be logged. This OurJSONLogRecord still surfaces the undocumented assumption, "Oh, we need the entire JSON document, we didn't realise anybody would want to stream data" whereas the Write trait does not.

tialaramex 4y ago

> the library authors had explicitly marked their type as `impl std::io::Write` without understanding why that wasn't appropriate. :( Name the library so people can avoid it? Or did they fix this and drop broken versions?

ezrast 4y ago
It's the SDK for Fastly's edge computing service, so not something you're likely to run into without getting paid for it. And no, still has the brokenness, still has the internal comment warning their own engineers not to do it wrong[1]. I'm doubtful that my support ticket about the issue ever made it to anyone who actually knows Rust. (edit to add: the feature was in beta at the time and we were early adopters, so Support not knowing how to field issues about it was sorta understandable. Their engineers seemed cool when I had the chance to talk to them) [1] https://docs.rs/fastly/0.8.5/src/fastly/log.rs.html#171 https://docs.rs/fastly/0.8.5/src/fastly/log.rs.html#171
- tialaramex 4y ago
  That Fastly documentation doesn't read as though it cares about JSON. Are you sure the JSON requirement isn't in some other system?
  - ezrast 4y ago
    The logging utilities are just a frontend to a bunch of supported logging/tracing providers, many but not all of which accept plain text. So yes, it was technically a separate system (Honeycomb) doing the rejection, but one that was blessed by the vendor. And the only debugging facility I had available, which was duplicating the payload to stdout and tailing that, masked the issue because stdout is an actual stream. And to be clear, the system would be broken for the plaintext providers too, just more obviously broken as you'd presumably have a bunch of JSON tokens (or segments of a format string or whatever) show up as individual log lines.

actionfromafar 4y ago

I find it so interesting that there is such a huge chasm between "what we know can be done" and "what we do in practice in industry". How can it be that in so many programming languages, adhering to a contract means nothing but "these methods have similar calling signatures" and "I pinky swear I implement the semantics of std::io::Write " ? Haven't the Ocaml people entered the next metaphysical level of understanding? When they speak of "types" they don't mean "uint32_t" or a class.

Zababa 4y ago
What does this have to do with OCaml?
- actionfromafar 4y ago
  They seem to operate at much higher level of abstraction. They mean something more profound when they speak of "types", which I don't understand. Also Ada seems interesting, with more spelled out contracts.
  - Zababa 4y ago
    That depends on the types. You can have something as basic as "int", something a bit more complex like arrays, or something even more complex like GADTs https://v2.ocaml.org/manual/gadts.html https://v2.ocaml.org/manual/gadts.html