Skip to content

Commit

Permalink
fix(orchestration): errors should have error messages (#9593)
Browse files Browse the repository at this point in the history
closes: #XXXX
refs: #9519

## Description

While trying to debug #9519 , I'm seeing errors like
```
Error#20: {"type":1,"data":"CmgKIy9jb3Ntb3Muc3Rha2luZy52MWJldGExLk1zZ0RlbGVnYXRlEkEKGFVOUEFSU0FCTEVfQ0hBSU5fQUREUkVTUxISYWdvcmljMXZhbG9wZXJmdWZ1GhEKBXVmbGl4EggxMDAwMDAwMA==","memo":""}
  at parseTxPacket (file:///Users/markmiller/src/ongithub/agoric/agoric-sdk/packages/orchestration/src/utils/packet.js:87:14)
```
which only has the data without any message text saying what's wrong with the data. While fixing, also switched to a `Fail` to give us control over redaction. I made the conservative assumption that to not unredact the actual packet data, because I don't know why it should be public on the error even in non-debugging scenarios. If it should be unredacted, I can easily change to `${q(response)}`. Reviewers, please advise.

Note that either way, the unredacted info will still show up in error logs, as above, and will still even show up on the error object in standard debugging configurations.

### Security Considerations
Possibly repaired an unintended disclosure of info that should have been redacted, though the main motivation was adding an informative error message.

### Scaling Considerations
none
### Documentation Considerations
none
### Testing Considerations
none
### Upgrade Considerations
none
  • Loading branch information
erights authored Jun 28, 2024
1 parent a3826e9 commit 5b2d338
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/orchestration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"@agoric/zoe": "^0.26.2",
"@agoric/zone": "^0.2.2",
"@endo/base64": "^1.0.5",
"@endo/errors": "^1.2.2",
"@endo/far": "^1.1.2",
"@endo/marshal": "^1.5.0",
"@endo/patterns": "^1.4.0"
Expand Down
3 changes: 2 additions & 1 deletion packages/orchestration/src/utils/packet.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Fail } from '@endo/errors';
import { TxBody } from '@agoric/cosmic-proto/cosmos/tx/v1beta1/tx.js';
import { Any } from '@agoric/cosmic-proto/google/protobuf/any.js';
import { RequestQuery } from '@agoric/cosmic-proto/tendermint/abci/types.js';
Expand Down Expand Up @@ -84,7 +85,7 @@ export function parseTxPacket(response) {
const { result, error } = JSON.parse(response);
if (result) return result;
else if (error) throw Error(error);
else throw Error(response);
else throw Fail`expected either result or error: ${response}`;
}
harden(parseTxPacket);

Expand Down
4 changes: 2 additions & 2 deletions packages/orchestration/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ test('makeICQConnection returns an ICQConnection', async t => {
),
]),
),
{ message: /"data":"(.*)"memo":""/ },
{ message: /\\"data\\":\\"(.*)\\"memo\\":\\"\\"/ },
'TODO do not use echo connection',
);
});
Expand Down Expand Up @@ -118,7 +118,7 @@ test('makeAccount returns a ChainAccount', async t => {
);
await t.throwsAsync(
heapVowTools.when(E(account).executeEncodedTx([delegateMsg])),
{ message: /"type":1(.*)"data":"(.*)"memo":""/ },
{ message: /\\"type\\":1(.*)\\"data\\":\\"(.*)\\"memo\\":\\"\\"/ },
'TODO do not use echo connection',
);

Expand Down
4 changes: 2 additions & 2 deletions packages/orchestration/test/utils/packet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ test('parseTxPacket', t => {
t.throws(
() => parseTxPacket('{"foo":"bar"}'),
{
message: '{"foo":"bar"}',
message: 'expected either result or error: "{\\"foo\\":\\"bar\\"}"',
},
'returns original string as error if `result` is not found',
);
Expand Down Expand Up @@ -187,7 +187,7 @@ test('parseQueryPacket', t => {
t.throws(
() => parseQueryPacket('{"foo":"bar"}'),
{
message: '{"foo":"bar"}',
message: 'expected either result or error: "{\\"foo\\":\\"bar\\"}"',
},
'throws an error if `result` is not found',
);
Expand Down

0 comments on commit 5b2d338

Please sign in to comment.