Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve log API #19

Open
pq opened this issue Sep 30, 2018 · 5 comments
Open

improve log API #19

pq opened this issue Sep 30, 2018 · 5 comments
Labels
api enhancement New feature or request
Milestone

Comments

@pq
Copy link
Owner

pq commented Sep 30, 2018

Follow-up from #16 (comment), minimally we should add:

@devoncarew: would 2 calls be sufficient?:

void log(MessageCallback messageCallback, {Object data, toEncodable(object)});

void logError(MessageCallback messageCallback, {Object error, toEncodable(object), StackTrace stackTrace});

Or do you think there's benefit in making a distinction between log and logData?

@pq pq added the enhancement New feature or request label Sep 30, 2018
@devoncarew
Copy link
Contributor

devoncarew commented Sep 30, 2018

W/ logData, we could tell the API user that there were some conventions about how to transmit the data; that we'd expect it to be jsonable, and would send over the object encoded as a json string.

Putting the data object as a json encoded string in the error: field is pretty prescriptive however. Perhaps have log() - that can take an optional data: param - and have a separate logDataAsJson(), which does the prescriptive encoding?

void log(MessageCallback messageCallback, {Object data, toEncodable(object)});

looks good!

void logError(MessageCallback messageCallback, {Object error, toEncodable(object), StackTrace stackTrace});

perhaps drop the toEncodable here? I think in this case we would not try and encode the object, but would just pass the reference as is into the dart:developer call.

@pq
Copy link
Owner Author

pq commented Oct 1, 2018

What do we know about constraints on the error object in the developer:log API? I was working on the assumption that the error object would ultimately be encoded as json but realize now that may not be right!

@devoncarew
Copy link
Contributor

It's just sent as a VM protocol reference - you can then call service protocol methods to get more info about it. From the client, you basically just see the vm service protocol id for the object (foo/123).

@pq pq mentioned this issue Oct 1, 2018
@pq
Copy link
Owner Author

pq commented Oct 2, 2018

#20 adds data logging but we'll want to keep iterating.

food for thought from @devoncarew on that PR:

  • a dedicated logData call could be useful, esp. to indicate that we have a prescriptive way to write the data
  • if we're very perceptive of what kind of data we can log (Map<String, Object>?), I wonder if we need both LogDataCallback data and ToJsonEncodable toJsonEncodablecallbacks? The LogDataCallback could just return a Map, and we'd document that all the objects in it needed to be jsonable (or, string, maps, lists, and ints)?

👍

@pq pq added the api label Oct 16, 2018
@pq pq added this to the 1.0 milestone Oct 17, 2018
@pq
Copy link
Owner Author

pq commented Oct 22, 2018

We should hammer out what we minimally need for 1.0.

#38 adds params to allow for passing of level and stackTrace down to the developer calls.

What else (if anything) do we need for v1?

/cc @devoncarew

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants