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.

Sign up for GitHub

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

Jump to bottom

When pull fails it doesn't provide details #690

Closed
aboodman opened this issue Nov 15, 2021 · 4 comments · Fixed by #716
Closed

When pull fails it doesn't provide details #690

aboodman opened this issue Nov 15, 2021 · 4 comments · Fixed by #716
Assignees
@grgbkr

Comments

@aboodman
Copy link
Contributor

I hit this, as did a user. We set the cause of the exception, here:

replicache/src/puller.ts

Line 101 in 16d2597

this.cause = cause;

But this doesn't show up in the console anywhere so it's hard to know what's going wrong. Maybe we should include the causes's message in our message?

Or maybe we don't even need this PullError abstraction anymore.
@aboodman aboodman added the fixit label Nov 15, 2021
@arv
Copy link
Contributor

arv commented Nov 16, 2021

They just need to use a better browser =P

image

Maybe related chromium issue?

@aboodman
Copy link
Contributor Author

Wow! What browser is that.

But in any case, we can't build features that don't work right for the majority browser ;-).

@arv
Copy link
Contributor

arv commented Nov 16, 2021

I'm just thinking that Chrome will fix this in the near term so it might not be worth spending time on.

Also, the latest Node.js also prints cause for stack traces.

@aboodman
Copy link
Contributor Author

If it's going to get fixed by Chromium and pushed out to stable like tomorrow or this week sure. But otherwise, let's fix.

@grgbkr grgbkr self-assigned this Nov 19, 2021
@grgbkr grgbkr mentioned this issue Nov 19, 2021
Merged
grgbkr added a commit that referenced this issue Nov 19, 2021
@grgbkr
fix: Provide more details to logger when push or pull fails (#716)
186bb70 
### Problem
Push and pull error logging lacks sufficient detail to debug errors.

A pull failure currently logs to info (and a push error logs essentially the same):
`Pull returned: PullError: Failed to pull`

Not the most useful logging.  However, our `PushError` and `PullError` classes have a `cause?: Error` property with details on the underlying cause, it is just not logged.

### Solution
If the error is a `PushError` or `PullError`, log the cause.

Update log format to include stack traces for both the error, and cause.

Also update to use `error` instead of `info` logging.

Closes #690
arv pushed a commit that referenced this issue Nov 22, 2021
@grgbkr @arv
fix: Provide more details to logger when push or pull fails (#716)
7197c23 
### Problem
Push and pull error logging lacks sufficient detail to debug errors.

A pull failure currently logs to info (and a push error logs essentially the same):
`Pull returned: PullError: Failed to pull`

Not the most useful logging.  However, our `PushError` and `PullError` classes have a `cause?: Error` property with details on the underlying cause, it is just not logged.

### Solution
If the error is a `PushError` or `PullError`, log the cause.

Update log format to include stack traces for both the error, and cause.

Also update to use `error` instead of `info` logging.

Closes #690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@grgbkr grgbkr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Provide more details to logger when push or pull fails rocicorp/replicache
3 participants
@arv @aboodman @grgbkr