Skip to content

Avoid doing console.error #333

New issue

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

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

Already on GitHub? # to your account

Closed
satazor opened this issue Sep 2, 2020 · 3 comments · Fixed by #419
Closed

Avoid doing console.error #333

satazor opened this issue Sep 2, 2020 · 3 comments · Fixed by #419
Labels
good first issue Good for newcomers module/lib Related to the main source code status/no-issue-activity

Comments

@satazor
Copy link

satazor commented Sep 2, 2020

This is not really a bug, nor a feature request, so I haven't followed any of the templates.

It seems console.error is being used in several situations, e.g.:

console.error(e.errors);
.

In most scenarios, these console.error calls are unnecessary as errors will be thrown anyway, that should be handled by devs consuming this library. In other situations, errors aren't thrown, and these messages are useful. However, most devs are using custom loggers (like JSON loggers), and would like to be able to customize the logging behavior.

@lance lance added good first issue Good for newcomers module/lib Related to the main source code labels Sep 2, 2020
@lance
Copy link
Member

lance commented Sep 2, 2020

Thanks @satazor for opening this issue. It actually looks like there are only two calls to console.error() - the one you pointed out, and this one

console.error(`Unknown spec version ${version}. Default to ${Version.V1}`);

I think probably the one you note is the only one that should be removed, as the one in src/messages/http is informational and an exception isn't thrown.

@satazor
Copy link
Author

satazor commented Sep 2, 2020

@lance 👍

For informal logs, please consider adding the ability to specify the logger (maybe with the same interface as console), so that we can override with custom loggers.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2020

This issue is stale because it has been open 30 days with no activity.

lholmquist added a commit to lholmquist/sdk-javascript that referenced this issue Oct 7, 2020
* When receiving an event,  we should not default the specversion to 1.0 if no specverion field is detected.
* If no specversion is detected or the specversion does not conform to a currently supported version, we will now throw a ValidationError

This is a BREAKING_CHANGE

fixes cloudevents#332, cloudevents#333

Signed-off-by: Lucas Holmquist <lholmqui@redhat.com>
lholmquist added a commit to lholmquist/sdk-javascript that referenced this issue Oct 7, 2020
* When receiving an event,  we should not default the specversion to 1.0 if no specverion field is detected.
* If no specversion is detected or the specversion does not conform to a currently supported version, we will now throw a ValidationError

This is a BREAKING_CHANGE

fixes cloudevents#332, cloudevents#333

Signed-off-by: Lucas Holmquist <lholmqui@redhat.com>
lance added a commit to lance/sdk-javascript that referenced this issue May 21, 2021
Even if the specversion is totally invalid, we should not change the value
received in an incoming `Message`. Previously we defaulted to 1.0 if we did
not recognize the version number. This commit changes that, leaving the value
unmodified. We default to parsing this mystery event with the 1.0 spec. When
the event is validated with `event.validate()` we return `false`.

One additional small change to eliminate a prettier warning about `parer`
being previously declared.

Fixes: cloudevents#332
Fixes: cloudevents#333

Signed-off-by: Lance Ball <lball@redhat.com>
lance added a commit that referenced this issue May 25, 2021
Even if the specversion is totally invalid, we should not change the value
received in an incoming `Message`. Previously we defaulted to 1.0 if we did
not recognize the version number. This commit changes that, leaving the value
unmodified. We default to parsing this mystery event with the 1.0 spec. When
the event is validated with `event.validate()` we return `false`.

One additional small change to eliminate a prettier warning about `parer`
being previously declared.

Fixes: #332
Fixes: #333

Signed-off-by: Lance Ball <lball@redhat.com>
lholmquist pushed a commit to lholmquist/sdk-javascript that referenced this issue Jul 6, 2021
Even if the specversion is totally invalid, we should not change the value
received in an incoming `Message`. Previously we defaulted to 1.0 if we did
not recognize the version number. This commit changes that, leaving the value
unmodified. We default to parsing this mystery event with the 1.0 spec. When
the event is validated with `event.validate()` we return `false`.

One additional small change to eliminate a prettier warning about `parer`
being previously declared.

Fixes: cloudevents#332
Fixes: cloudevents#333

Signed-off-by: Lance Ball <lball@redhat.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
good first issue Good for newcomers module/lib Related to the main source code status/no-issue-activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants