'tahoe ls' on unknown objects: error message should be clearer #837

New issue

Closed

opened 2009-11-20 19:31:26 +00:00 by warner · 12 comments

warner commented

2009-11-20 19:31:26 +00:00

If you list a directory with tahoe ls, and it happens to
contain an object that your client doesn't recognize (say, a cap
type from the future, like DIR-IMM from a pre-1.6 client), the
child name is displayed normally. If you then try to list that
object, you get an error message (because your client doesn't
know how to download or interpret it), that looks like this:

% ./bin/tahoe ls alias:source-backups/Latest
Error during GET: 400 Bad Request GET unknown: can only do t=info, not t=json

We should make this error message more informative. The key word
in the message is "unknown", and means (from the point of view of
the code that emits it) that the IFilesystemNode object you
pointed at is of type UnknownNode (as opposed to
IFileNode or IDirectoryNode or something else it
knows how to handle).

The CLI side should probably spot the webapi error message
(if "unknown: can only do t=info" in data) and append
something helpful, like "This means you tried to list a
filesystem object that was unrecognized by your tahoe client,
probably because it was created by a newer version. Upgrading
your tahoe client to the latest version may enable you to use
this object."""

If you list a directory with `tahoe ls`, and it happens to contain an object that your client doesn't recognize (say, a cap type from the future, like DIR-IMM from a pre-1.6 client), the child name is displayed normally. If you then try to list that object, you get an error message (because your client doesn't know how to download or interpret it), that looks like this: ``` % ./bin/tahoe ls alias:source-backups/Latest Error during GET: 400 Bad Request GET unknown: can only do t=info, not t=json ``` We should make this error message more informative. The key word in the message is "unknown", and means (from the point of view of the code that emits it) that the `IFilesystemNode` object you pointed at is of type `UnknownNode` (as opposed to `IFileNode` or `IDirectoryNode` or something else it knows how to handle). The CLI side should probably spot the webapi error message (`if "unknown: can only do t=info" in data`) and append something helpful, like "This means you tried to list a filesystem object that was unrecognized by your tahoe client, probably because it was created by a newer version. Upgrading your tahoe client to the latest version may enable you to use this object."""

warner added the

labels 2009-11-20 19:31:26 +00:00

warner added this to the undecided milestone 2009-11-20 19:31:26 +00:00

daira modified the milestone from undecided to 1.6.0

2009-11-21 06:29:15 +00:00

daira self-assigned this 2009-12-20 01:10:36 +00:00

zooko modified the milestone from 1.6.0 to eventually

2010-01-26 15:44:33 +00:00

daira modified the milestone from eventually to 1.7.0

2010-02-01 19:41:43 +00:00

daira commented

2010-02-09 05:33:45 +00:00

./bin/tahoe ls alias:source-backups/Latest

Error during GET: 400 Bad Request GET unknown: can only do t=info, not t=json

Unfortunately this has regressed in 1.6 to printing no message at all. This is an unanticipated consequence of the patch for #833 having implemented t=json for unknown nodes. It happens because tahoe_ls.py no longer gets an error from the GET request, but the nodetype in the returned JSON is "unknown", which causes the code to [fall through the nodetype check]source:src/allmydata/scripts/tahoe_ls.py?rev=4142#L50 with children still set to a empty dict.

OTOH, the fact that t=json now works for unknown nodes makes it easy to fix this to display all the information we do know about the node. I have a patch to make it do that:

$ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file
?r-- ? Jan 28 22:16 immutable-future-file

This listing included unknown objects. Using a webapi server that supports
a later version of Tahoe may help.

>`./bin/tahoe ls alias:source-backups/Latest` >`Error during GET: 400 Bad Request GET unknown: can only do t=info, not t=json` Unfortunately this has regressed in 1.6 to printing no message at all. This is an unanticipated consequence of the patch for #833 having implemented `t=json` for unknown nodes. It happens because `tahoe_ls.py` no longer gets an error from the GET request, but the nodetype in the returned JSON is `"unknown"`, which causes the code to [fall through the nodetype check]source:src/allmydata/scripts/tahoe_ls.py?rev=4142#L50 with `children` still set to a empty dict. OTOH, the fact that `t=json` now works for unknown nodes makes it easy to fix this to display all the information we do know about the node. I have a patch to make it do that: ``` $ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file ?r-- ? Jan 28 22:16 immutable-future-file This listing included unknown objects. Using a webapi server that supports a later version of Tahoe may help. ```

daira commented

2010-02-09 05:40:56 +00:00

Attachment improve-ls-for-unknown-objects-darcspatch.txt (52860 bytes) added

Improve behaviour of 'tahoe ls' for unknown objects (currently lacks tests)

**Attachment** improve-ls-for-unknown-objects-darcspatch.txt (52860 bytes) added Improve behaviour of 'tahoe ls' for unknown objects (currently lacks tests)

improve-ls-for-unknown-objects-darcspatch.txt

52 KiB

daira commented

2010-02-09 05:45:25 +00:00

Note that the patch also allows the filename to be displayed when the argument to tahoe ls is a path to a known file:

$ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/test.txt
-r-- 22 Jan 23 21:37 test.txt

Previously this would only have printed

-r-- 22 Jan 23 21:37

unless the URI were specified with ":./", e.g. tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq:./test.txt

Note that the patch also allows the filename to be displayed when the argument to `tahoe ls` is a path to a known file: ``` $ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/test.txt -r-- 22 Jan 23 21:37 test.txt ``` Previously this would only have printed ``` -r-- 22 Jan 23 21:37 ``` unless the URI were specified with ":./", e.g. `tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq:./test.txt`

zooko commented

2010-02-15 03:28:56 +00:00

This is strictly a bug-fix, and it is a regression in v1.6.0, so it is appropriate for v1.6.1.

zooko modified the milestone from 1.7.0 to 1.6.1

2010-02-15 03:28:56 +00:00

kevan commented

2010-02-16 00:53:35 +00:00

Owner

You have some trailing whitespace on line 66 of tahoe_ls.py

Unless I'm mistaken, it isn't necessary to set has_unknowns = True on line 65 -- the same comparison (effectively) and the same operation are performed on line 113, in the formatting loop.

It isn't immediately clear to me (as someone who has never viewed this ticket before) what the question marks in the output mean. Take, for example,

$ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file
?r-- ? Jan 28 22:16 immutable-future-file

After reading your patch, I think that the first one means "I can't determine whether or not this is a dirnode", and the second one means "I can't determine the size of this object". Is that right? tahoe ls --help doesn't display anything in particular about the format of that output, though, instead showing:

pathos:tahoe-1.6 kacarstensen$ tahoe ls --help
Usage:  tahoe <command> [command options] ls [options]
Options:
  -q, --quiet             Operate silently.
  -V, --version           Display version numbers and exit.
      --version-and-path  Display version numbers and paths to their locations
                          and exit.
  -l, --long              Use long format: show file sizes, and timestamps
      --uri               Show file/directory URIs
      --readonly-uri      Show readonly file/directory URIs
  -F, --classify          Append '/' to directory names, and '*' to mutable
      --json              Show the raw JSON output
  -d, --node-directory=   Look here to find out which Tahoe node should be used
                          for all operations. The directory should either
                          contain a full Tahoe node, or a file named node.url
                          which points to some other Tahoe node. It should also
                          contain a file named private/aliases which contains
                          the mapping from alias name to root dirnode URI.
                          [default: ~/.tahoe]
  -u, --node-url=         URL of the tahoe node to use, a URL like
                          "http://127.0.0.1:3456". This overrides the URL found
                          in the --node-directory .
      --dir-cap=          Which dirnode URI should be used as the 'tahoe' alias.
      --help              Display this help and exit.

List the contents of some portion of the grid.

Would you consider amending that --help text as a part of this ticket? A general description of the long format would be great, but even a blurb about what ?s in the output mean would help. It's arguably out of scope for this ticket, I guess.

Your changes to get_alias look good. Are you aware of any other CLI commands that will break somehow if allowed to accept arguments of the URI:blah/foo form (since so many of them use get_alias)?

You have some trailing whitespace on line 66 of `tahoe_ls.py` Unless I'm mistaken, it isn't necessary to set `has_unknowns = True` on line 65 -- the same comparison (effectively) and the same operation are performed on line 113, in the formatting loop. It isn't immediately clear to me (as someone who has never viewed this ticket before) what the question marks in the output mean. Take, for example, ``` $ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file ?r-- ? Jan 28 22:16 immutable-future-file ``` After reading your patch, I think that the first one means "I can't determine whether or not this is a dirnode", and the second one means "I can't determine the size of this object". Is that right? `tahoe ls --help` doesn't display anything in particular about the format of that output, though, instead showing: ``` pathos:tahoe-1.6 kacarstensen$ tahoe ls --help Usage: tahoe <command> [command options] ls [options] Options: -q, --quiet Operate silently. -V, --version Display version numbers and exit. --version-and-path Display version numbers and paths to their locations and exit. -l, --long Use long format: show file sizes, and timestamps --uri Show file/directory URIs --readonly-uri Show readonly file/directory URIs -F, --classify Append '/' to directory names, and '*' to mutable --json Show the raw JSON output -d, --node-directory= Look here to find out which Tahoe node should be used for all operations. The directory should either contain a full Tahoe node, or a file named node.url which points to some other Tahoe node. It should also contain a file named private/aliases which contains the mapping from alias name to root dirnode URI. [default: ~/.tahoe] -u, --node-url= URL of the tahoe node to use, a URL like "http://127.0.0.1:3456". This overrides the URL found in the --node-directory . --dir-cap= Which dirnode URI should be used as the 'tahoe' alias. --help Display this help and exit. List the contents of some portion of the grid. ``` Would you consider amending that `--help` text as a part of this ticket? A general description of the long format would be great, but even a blurb about what `?`s in the output mean would help. It's arguably out of scope for this ticket, I guess. Your changes to `get_alias` look good. Are you aware of any other CLI commands that will break somehow if allowed to accept arguments of the URI:blah/foo form (since so many of them use `get_alias`)?

daira commented

2010-02-20 06:11:11 +00:00

Replying to kevan:

You have some trailing whitespace on line 66 of tahoe_ls.py

Fixed.

Unless I'm mistaken, it isn't necessary to set has_unknowns = True on line 65 -- the same comparison (effectively) and the same operation are performed on line 113, in the formatting loop.

Agreed, fixed.

It isn't immediately clear to me (as someone who has never viewed this ticket before) what the question marks in the output mean. Take, for example,
$ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file
?r-- ? Jan 28 22:16 immutable-future-file
After reading your patch, I think that the first one means "I can't determine whether or not this is a dirnode", and the second one means "I can't determine the size of this object". Is that right?

Yes. tahoe ls --help now ends with:

    List the contents of some portion of the grid.

    When the -l or --long option is used, each line is shown in the
    following format:

    drwx <size> <date/time> <name in this directory>

    where each of the letters on the left may be replaced by '-'.
    If 'd' is present, it indicates that the object is a directory.
    If the 'd' is replaced by a '?', the object type is unknown.
    'rwx' is a Unix-like permissions mask: if the mask includes 'w',
    then the object is writable through its link in this directory.
    The 'x' is a legacy of Unix filesystems. In Tahoe it is used
    only to indicate that the contents of a directory can be listed.

    Directories have no size, so their size field is shown as '-'.
    Otherwise the size of the file, when known, is given in bytes.
    The size of mutable files or unknown objects is shown as '?'.

    The date/time shows when this link in the Tahoe filesystem was
    last modified.

Your changes to get_alias look good. Are you aware of any other CLI commands that will break somehow if allowed to accept arguments of the URI:blah/foo form (since so many of them use get_alias)?

No, I don't think there is anything that depends on the syntax of an URI in that way.

Replying to [kevan](/tahoe-lafs/trac/issues/837#issuecomment-374500): > You have some trailing whitespace on line 66 of `tahoe_ls.py` Fixed. > Unless I'm mistaken, it isn't necessary to set `has_unknowns = True` on line 65 -- the same comparison (effectively) and the same operation are performed on line 113, in the formatting loop. Agreed, fixed. > It isn't immediately clear to me (as someone who has never viewed this ticket before) what the question marks in the output mean. Take, for example, > > ``` > $ tahoe ls -l URI:DIR2:bntvvn2bf37vwmrd3ttbygt4s4:qymwa2lcvhjvi3hnamxew6ywro5jmj7zfnspmrpqv3n7faibprlq/immutable-future-file > ?r-- ? Jan 28 22:16 immutable-future-file > ``` > > After reading your patch, I think that the first one means "I can't determine whether or not this is a dirnode", and the second one means "I can't determine the size of this object". Is that right? Yes. `tahoe ls --help` now ends with: ``` List the contents of some portion of the grid. When the -l or --long option is used, each line is shown in the following format: drwx <size> <date/time> <name in this directory> where each of the letters on the left may be replaced by '-'. If 'd' is present, it indicates that the object is a directory. If the 'd' is replaced by a '?', the object type is unknown. 'rwx' is a Unix-like permissions mask: if the mask includes 'w', then the object is writable through its link in this directory. The 'x' is a legacy of Unix filesystems. In Tahoe it is used only to indicate that the contents of a directory can be listed. Directories have no size, so their size field is shown as '-'. Otherwise the size of the file, when known, is given in bytes. The size of mutable files or unknown objects is shown as '?'. The date/time shows when this link in the Tahoe filesystem was last modified. ``` > Your changes to `get_alias` look good. Are you aware of any other CLI commands that will break somehow if allowed to accept arguments of the URI:blah/foo form (since so many of them use `get_alias`)? No, I don't think there is anything that depends on the syntax of an URI in that way.

daira commented

2010-02-20 06:28:58 +00:00

Attachment improve-ls-for-unknown-objects-2-darcspatch.txt (54095 bytes) added

Improve behaviour of 'tahoe ls' for unknown objects, addressing kevan's comments (still no tests)

**Attachment** improve-ls-for-unknown-objects-2-darcspatch.txt (54095 bytes) added Improve behaviour of 'tahoe ls' for unknown objects, addressing kevan's comments (still no tests)

improve-ls-for-unknown-objects-2-darcspatch.txt

53 KiB

zooko commented

2010-02-21 05:47:00 +00:00

Unsetting the review-needed tag until there are tests. (By the way, this sounds like a good improvement!)

Unsetting the `review-needed` tag until there are tests. (By the way, this sounds like a good improvement!)

daira commented

2010-02-24 03:12:38 +00:00

Attachment improve-ls-for-unknown-objects-3-darcspatch.txt (58166 bytes) added

Improve behaviour of 'tahoe ls' for unknown objects (with tweak to 'tahoe ls --help' output)

**Attachment** improve-ls-for-unknown-objects-3-darcspatch.txt (58166 bytes) added Improve behaviour of 'tahoe ls' for unknown objects (with tweak to 'tahoe ls --help' output)

improve-ls-for-unknown-objects-3-darcspatch.txt

57 KiB

daira commented

2010-02-24 03:14:35 +00:00

Attachment test-ls-for-unknown-objects-darcspatch.txt (57943 bytes) added

Test behaviour of 'tahoe ls' for unknown objects (#837)

**Attachment** test-ls-for-unknown-objects-darcspatch.txt (57943 bytes) added Test behaviour of 'tahoe ls' for unknown objects (#837)

test-ls-for-unknown-objects-darcspatch.txt

57 KiB

kevan commented

2010-02-24 05:41:39 +00:00

Owner

The tests look good to me. This patch changes the semantics of get_alias, and what is printed when using ls on an unknown file, and you test for both of those changes, so I don't think you're missing any tests, and the tests fail when your other patch isn't applied. I'm marking this as 'reviewed', since I've reviewed the other patch and am confident that they're both fine.

The tests look good to me. This patch changes the semantics of `get_alias`, and what is printed when using `ls` on an unknown file, and you test for both of those changes, so I don't think you're missing any tests, and the tests fail when your other patch isn't applied. I'm marking this as 'reviewed', since I've reviewed the other patch and am confident that they're both fine.