WIP encapsulate marshal error kludgery #997
Conversation
erights
force-pushed
the
4310-markm-refactor-marshalSaveError
branch
2 times, most recently
from
db9b58f
to
3503e28
Compare
| errorId, | ||
| message: `${err.message}`, | ||
| name: `${err.name}`, |
There was a problem hiding this comment.
Nit, errorId instead of simply id implies the latter messages should be qualified errorMessage and errorName. I think id would suffice.
There was a problem hiding this comment.
Given that I'm now trying to stay compat with existing clients, I don't see a good and painless way to change this.
packages/captp/src/captp.js
Outdated
| marshalSaveError: makeMarshalSaveError({ | ||
| marshalName: `captp:${ourId}`, | ||
| // TODO Temporary hack. | ||
| // See https://github.com/Agoric/agoric-sdk/issues/2780 | ||
| errorIdNum: 20000, | ||
| }), |
There was a problem hiding this comment.
If this is the breaking change, I think we could coerce {marshalName, errorIdNum} up to an error saver for a smoother migration.
There was a problem hiding this comment.
I have now engineered things for a smoother migration. But I don't understand how my changes relate to your suggestion.
erights
force-pushed
the
4310-markm-refactor-marshalSaveError
branch
5 times, most recently
from
594af95
to
f7cb976
Compare
|
I have now enhanced it to be compatible with users of the old interface. The adaptation was ugly, but worth it avoid a more painful upgrade dance. It is now ready for review. |
erights
force-pushed
the
4310-markm-refactor-marshalSaveError
branch
2 times, most recently
from
37f3a60
to
db15585
Compare
|
ping |
erights
force-pushed
the
4310-markm-refactor-marshalSaveError
branch
from
051ed0a
to
bd234a9
Compare
|
If we try to do this again, we should start a new PR. Closing this one as fatally stale. |
DO NOT MERGE: Based on an ancient master, and probably needs redoing anyway.
Fixes Agoric/agoric-sdk#4310
Please review at this stage, even though it is a WIP Draft. This is because of the need to manage the version skew between the endo repository and agoric-sdk. The API change in this PR is an incompatible break, which needs coordination this PR does not yet provide. But I'd like to postpone thinking about that until we agree on an API that is otherwise good.