Possible bug with Storage.open

Hi there!

Before opening a bug on the tracker, I wanted to double check this behaviour.
This code opens a file in append mode, and writes some text in it.

const Storage = require('Storage')
const name = 'test'
Storage.erase(name)
const file = Storage.open(name, 'a')
console.log('-> files', Storage.list())
// file not in the list
file.write('whatever')
console.log('-> files', Storage.list())
// file exists, but has a \1 octal escape sequence appended to its name

this gives:

 ____                 _
|  __|___ ___ ___ _ _|_|___ ___
|  __|_ -| . |  _| | | |   | . |
|____|___|  _|_| |___|_|_|_|___|
         |_| espruino.com
 2v04 (c) 2019 G.Williams
>-> files [
  "+mclock",
  "-mclock",
  "*mclock",
  "+setting",
  "-setting",
  "=setting",
  "*setting",
  "+sbt",
  "=sbt",
  "+sbat",
  "=sbat",
  "+files",
  "-files",
  "*files",
  ".trishig",
  ".bootcde",
  "@setting",
 ]
-> files [
  "+mclock",
  "-mclock",
  "*mclock",
  "+setting",
  "-setting",
  "=setting",
  "*setting",
  "+sbt",
  "=sbt",
  "+sbat",
  "=sbat",
  "+files",
  "-files",
  "*files",
  ".trishig",
  ".bootcde",
  "@setting",
  "test\1"
 ]

This \1 at the end: is it expected?

Strange, I don't see open method documented here https://www.espruino.com/Reference#Storage and the write method example doesn't show that open call is needed.

EDIT:
Oh, but I see it here in source https://github.com/espruino/Espruino/blob/master/src/jswrap_storage.c#L287 so the generated documentation needs refresh?

oh maybe the \1 is chunk name or whatever (i.e each append create another numbered one)? The limit is 8 characters here https://github.com/espruino/Espruino/blob/master/src/jswrap_storage.c#L85 and 7 here https://github.com/espruino/Espruino/blob/master/src/jswrap_storage.c#L294

EDIT: oh yes, here it is clear https://github.com/espruino/Espruino/blob/master/src/jswrap_storage.c#L612

Also this may bite someone https://github.com/espruino/Espruino/blob/master/src/jswrap_storage.c#L395 "you should not write character code 255 ("\xFF") to these files" so these are basically text files, not binary.

This \1 at the end: is it expected?

Yes, that's expected. As you write more, you'll get a \2/etc. It's to allow the file to grow in size past that which is originally allocated. The files aren't really readable in the same way as other files.

I'll add something to the StorageFile docs.

so the generated documentation needs refresh?

The generated docs match the current stable firmware release - and .open is in one of the cutting edge builds :)

you should not write character code 255

It might - but honestly if you're writing binary files rather than text you're probably more likely to have read the docs and seen that. If you can think of other solutions for this I'm happy to look at them, but this seemed like the most stable way of handling 'growing' a file.

I see now that it is not just some comment but part of documentation, so that's fine. It is just confusion caused by not seeing generated documentation yet.

FF could be solved by having some escape character and then escaping it and some FF replacement value similar to e.g. what SLIP does. But that would work only if you read it by characters and make a copy, not when results point directly to flash data. If these are meant to be text files it is probably not worth it.

FF could be solved by having some escape character

Good point. If it becomes an issue then that could be added. Potentially there could also be a check & exception in write just to make sure...

Thanks for the clarification folks.

This final character makes Puck.eval() failing when transfering these strings:

Puck.eval('require("Storage").list()')

will fail on:

Unable to decode "[\"+mclock\",\"-mclock\",\"*mclock\",\"+setting\",\"-setting\",\"=setting\",\"*setting\",\"+astroid\",\"-astroid\",\"*astroid\",\"+gpstime\",\"-gpstime\",\"*gpstime\",\"+compass\",\"-compass\",\"*compass\",\"+sbt\",\"=sbt\",\"+sbat\",\"=sbat\",\"+files\",\"-files\",\"*files\",\"+hrm\",\"-hrm\",\"*hrm\",\"+gpsinfo\",\"-gpsinfo\",\"*gpsinfo\",\".trishig\",\".bootcde\",\"+speedo\",\"-speedo\",\"*speedo\",\"@setting\",\"@activi\\1\",\"-activit\",\"+activit\",\"test\\1\"]\r", got SyntaxError: Unexpected number in JSON at position 357

Having chosen an octal escape sequence make it hard to (de)serialize.

To give a bit of context: I would like the ability to list files present on the board.
I though it would be a small addition ton BangleApps to display such list, and allow to download them.

I've mitigated the encoding issue:

      // this is on the BangleApp:
      Puck.eval('require("Storage").list().map(encodeURIComponent)', (files,err) => {
        if (files===null) return reject(err || "");
        files = files.map(decodeURIComponent)
        console.log("listFiles", files);
        resolve(files)
      });

Thanks for the update. I'm surprised this is the first time it's been hit. It looks like "\1"/etc are not supported in JSON(!) and you have to use \u0001 for everything.

I guess the solution for now might be to add a regex to Puck.eval to convert the characters, but long-term I guess JSON.stringify should be spec compliant even if it uses way more memory :(

Issues filed at https://github.com/espruino/Espruino/issues/1737 and https://github.com/espruino/EspruinoWebTools/issues/1

I lost quite some time trying to replace octal sequence with regex... with no luck, hence why I used encodeURIComponent instead.

I'll be very interested to learn about the solution, would some regex wizard lit my lamp.

...regex lamps are domed to glow dim... sorry, myself not much more than smoldering regex wick..

@Gordon just to clarify, once the file increases in size and starts getting the shard marker in the end we have to use the shard marker in the name when we do a read, correct?

// say I have written a big file called 'myfile'
var s = require("Storage");
// print(s.read("myfile")); // would return undefined
print(s.read("myfile\1")); // returns the first block of the file.

I am getting this behaviour, I am still on the default beta version firmware that came with my KS Beta unit.

once the file increases in size and starts getting the shard marker in the end we have to use the shard marker in the name when we do a read, correct?

No... If you use StorageFile.write to create it, you need to use StorageFile.read to read it - use the same 7 character filename in each case and you should be fine!

@Gordon thanks... works perfectly with StorageFile. Silly me 🥴, I was trying to use Storage.read or something.