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

Normalize error message grammar (TS/JS) #25269

Open
irbull opened this issue Aug 28, 2024 · 3 comments
Open

Normalize error message grammar (TS/JS) #25269

irbull opened this issue Aug 28, 2024 · 3 comments

Comments

@irbull
Copy link
Contributor

irbull commented Aug 28, 2024

There are a variety of different ways errors in Deno's TS/JS layer are raised. Some are sentence case, others not. Some end with a period, others do not. Some provide the current state of the system, others do not.

Without a standard error guide, it makes it hard for new contributors to determine how to format error message. Furthermore, without a standard error guide, it makes it hard for users to know what to expect.

We just went through the std and normalized the errors there. We should do the same with the JavaScript / TypeScript code in the cli / runtime. There is an issue about improving error messages in the cli [1] which is orthogonal to this.

I think we should start with the Error Guide used in the std and see if that's a good fit for the JavaScript / TypeScript code in the cli [2].

[1] #24699
[2] denoland/std#5574

irbull added a commit to irbull/deno-docs that referenced this issue Aug 28, 2024
Adds a section to the style guide that explains how to write error
messages for the TypeScript / JavaScript parts of the Deno codebase.

This style guide was used in the `std` and seems to be a good fit for
the runtime as well. It can be updated as we refactor the existing
error messages.

denoland/deno#25269
irbull added a commit to irbull/deno-docs that referenced this issue Aug 28, 2024
Adds a section to the style guide that explains how to write error
messages for the TypeScript / JavaScript parts of the Deno codebase.

This style guide was used in the `std` and seems to be a good fit for
the runtime as well. It can be updated as we refactor the existing
error messages.

denoland/deno#25269
@irbull
Copy link
Contributor Author

irbull commented Aug 29, 2024

I wrote a small Deno script to scan the directories of the deno/deno project and determine which folders have JS/TS files and which of those files throw . I excluded /node_modules/, /testdata/, /tests/, ./github/, /bench, and /tsc. Here is what I found:

Folder Throws Status PR
./cli/js 32 #25406
./ext/broadcast_channel 2 No obvious problems found
./ext/cache 5 #25310
./ext/canvas 2 #25310
./ext/console 3 #25301
./ext/cron 5 #25300
./ext/crypto 226 #25440
./ext/fetch 46 #25374
./ext/ffi 5 #25310
./ext/fs 14 #25414
./ext/http 37 #25496
./ext/io 0 No obvious problems found
./ext/kv 24 #25500
./ext/net 26
./ext/node/polyfills 288
./ext/node/polyfills/_fs 73
./ext/node/polyfills/_process 3
./ext/node/polyfills/_util 20
./ext/node/polyfills/assert 1
./ext/node/polyfills/dns 0 No obvious problems found
./ext/node/polyfills/fs 0 No obvious problems found
./ext/node/polyfills/internal 31
./ext/node/polyfills/internal/crypto 91
./ext/node/polyfills/internal/dns 6
./ext/node/polyfills/internal/fs 0 No obvious problems found
./ext/node/polyfills/internal/process 0 No obvious problems found
./ext/node/polyfills/internal/test 0 No obvious problems found
./ext/node/polyfills/internal/util 2
./ext/node/polyfills/internal/util/parse_args 9
./ext/node/polyfills/internal_binding 9
./ext/node/polyfills/path 9
./ext/node/polyfills/readline 0 No obvious problems found
./ext/node/polyfills/stream 0 No obvious problems found
./ext/node/polyfills/timers 0 No obvious problems found
./ext/node/polyfills/util 0 No obvious problems found
./ext/url 3
./ext/web 138
./ext/webgpu 35 #25719
./ext/webidl 38 #25625
./ext/websocket 14 #25622
./ext/websocket/autobahn 0 No obvious problems found
./ext/webstorage 0 No obvious problems found
./runtime/examples/extension 0 No obvious problems found
./runtime/js 46 #25563
./tools 17 Folder only contains internal tools
./tools/napi 0 No obvious problems found
./tools/release 4

Script: https://gist.github.com/irbull/a16232d1a96ee3023501baf48b17a7e9

@oles
Copy link

oles commented Aug 29, 2024

I'd say something like https://github.com/denoland/std/blob/main/.github/CONTRIBUTING.md#error-messages would be a nice addition to the Deno Style Guide!

Good job on the Standard Library, @irbull!

@irbull
Copy link
Contributor Author

irbull commented Aug 29, 2024

Thanks for the kind words @oles. I've proposed a PR to the deno style guide with the same formatting used in the std. denoland/docs#759

I'm putting together a few change-sets that update the JS/TS code in the runtime so we can see how we feel about that here too.

irbull added a commit to irbull/deno that referenced this issue Aug 29, 2024
Aligns the error messages in the cron extension to be in-line with the
Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Aug 29, 2024
Aligns the error messages in the console extension to be in-line with
the Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Aug 30, 2024
Aligns the error messages in the console extension to be in-line with
the Deno style guide. This change-set aligns the messages in:
 - ext/cache
 - ext/canvas
 - ext/ffi

denoland#25269
marvinhagemeister pushed a commit that referenced this issue Sep 2, 2024
Aligns the error messages in the console extension to be in-line with
the Deno style guide.

#25269
irbull added a commit to irbull/deno that referenced this issue Sep 3, 2024
Aligns the error messages in the ext/fetch folder to be in-line with the
Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 3, 2024
Aligns the error messages in the cli/js folder to be in-line with the
Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 3, 2024
Aligns the error messages in the cli/js folder to be in-line with the
Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 4, 2024
Aligns the error messages in the ext/fs folder to be in-line with the
Deno style guide.

denoland#25269
marvinhagemeister pushed a commit that referenced this issue Sep 4, 2024
Aligns the error messages in the ext/fetch folder to be in-line with the
Deno style guide.

#25269
irbull added a commit to irbull/deno that referenced this issue Sep 5, 2024
Aligns the error messages in the ext/crypto folder to be in-line with
the Deno style guide.

denoland#25269
marvinhagemeister pushed a commit that referenced this issue Sep 5, 2024
Aligns the error messages in the ext/crypto folder to be in-line with
the Deno style guide.

#25269
marvinhagemeister pushed a commit that referenced this issue Sep 5, 2024
Aligns the error messages in the cron extension to be in-line with the
Deno style guide.

#25269
irbull added a commit to irbull/deno that referenced this issue Sep 6, 2024
Aligns the error messages in the ext/http and a few messages in the
ext/fetch folder to be in-line with the Deno style guide.

This change-set also removes some unnecessary checks in the 00_serve.ts.
These options were recently removed, so it doesn't make sense to check
for them anymore.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 7, 2024
Aligns the error messages in the ext/http and a few messages in the
ext/fetch folder to be in-line with the Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 7, 2024
Aligns the error messages in the ext/kv folder to be in-line with the
Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 10, 2024
Aligns the error messages in the runtime folder to be in-line with the
Deno style guide.

denoland#25269
marvinhagemeister pushed a commit that referenced this issue Sep 13, 2024
Aligns the error messages in the runtime folder to be in-line with the
Deno style guide.

#25269
irbull added a commit to irbull/deno that referenced this issue Sep 13, 2024
Aligns the error messages in the ext/websocket folder to be in-line with
the Deno style guide.

denoland#25269
irbull added a commit to irbull/deno that referenced this issue Sep 14, 2024
Aligns the error messages in the ext/webidl folder to be in-line with
the Deno style guide.

denoland#25269
nathanwhit pushed a commit that referenced this issue Sep 17, 2024
Aligns the error messages in the ext/websocket folder to be in-line with
the Deno style guide.

#25269
nathanwhit pushed a commit that referenced this issue Sep 19, 2024
Aligns the error messages in the ext/http and a few messages in the
ext/fetch folder to be in-line with the Deno style guide.

This change-set also removes some unnecessary checks in the 00_serve.ts.
These options were recently removed, so it doesn't make sense to check
for them anymore.

#25269
irbull added a commit to irbull/deno that referenced this issue Sep 19, 2024
Aligns the error messages in the ext/webgpu folder to be in-line with
the Deno style guide.

denoland#25269
marvinhagemeister pushed a commit that referenced this issue Sep 19, 2024
Aligns the error messages in the ext/webgpu folder to be in-line with
the Deno style guide.

#25269
satyarohith pushed a commit that referenced this issue Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants