e878ea9db4
This commit introduces a comprehensive set of improvements to the error handling and loading mechanism of the WebAssembly (WASM) module. The key changes include: - **Structured Error Handling:** Replaced generic string-based errors with a structured `WasmError` type in the Go WASM wrapper. This provides standardized error codes (`bad_request`, `not_found`, `conflict`) and clear messages, allowing JavaScript clients to handle errors programmatically. - **Isomorphic WASM Loader:** Refactored the JavaScript loader (`loader.js`) to be isomorphic, enabling it to run seamlessly in both browser and Node.js environments. The loader now detects the environment and uses the appropriate mechanism for loading the WASM binary and `wasm_exec.js`. - **Type Conversion Fix:** Resolved a panic (`panic: ValueOf: invalid value`) that occurred when returning `[]float64` slices from Go to JavaScript. A new `pointToJS` helper function now correctly converts these slices to `[]any`, ensuring proper data marshalling. - **Improved Smoke Test:** Enhanced the WASM smoke test (`smoke.mjs`) to verify the new structured error handling and to correctly handle the API's response format. - **Configuration Updates:** Updated the `.golangci.yml` configuration to be compatible with the latest version of `golangci-lint`. In addition to these changes, this commit also includes a new `AUDIT-ERROR-HANDLING.md` file, which documents the findings of a thorough audit of the project's error handling and logging practices. Co-authored-by: Snider <631881+Snider@users.noreply.github.com>
69 lines
3.8 KiB
Markdown
69 lines
3.8 KiB
Markdown
# Audit: Error Handling & Logging
|
|
|
|
This audit reviews the error handling and logging practices of the Poindexter Go library, focusing on the core `kdtree.go` implementation and the `wasm/main.go` wrapper.
|
|
|
|
## Error Handling
|
|
|
|
### Exception Handling
|
|
- [x] **Are exceptions caught appropriately?**
|
|
- **Finding:** Yes. As a Go library, it doesn't use exceptions but instead returns `error` types. The code diligently checks for and propagates errors. For instance, `NewKDTree` returns an error for invalid input, and callers are expected to handle it.
|
|
|
|
- [x] **Generic catches hiding bugs?**
|
|
- **Finding:** No. The library defines specific, exported error variables (e.g., `ErrEmptyPoints`, `ErrDimMismatch`) in `kdtree.go`. This allows consumers to programmatically check for specific error conditions using `errors.Is`, which is a best practice.
|
|
|
|
- [x] **Error information leakage?**
|
|
- **Finding:** Previously, the WASM wrapper at `wasm/main.go` would return raw Go error strings to the JavaScript client. This has been **remediated** by introducing a structured `WasmError` type with a `code` and `message`, preventing the leakage of internal implementation details.
|
|
|
|
### Error Recovery
|
|
- [x] **Graceful degradation?**
|
|
- **Finding:** Yes. The `KDTree` constructor attempts to use the `gonum` backend if requested, but it gracefully falls back to the `linear` backend if the `gonum` build tag is not present or if the backend fails to initialize. This ensures the library remains functional even without the optimized backend.
|
|
|
|
- [ ] **Retry logic with backoff?**
|
|
- **Finding:** Not applicable. This is a computational library, not a networked service, so retry logic is not relevant.
|
|
|
|
- [ ] **Circuit breaker patterns?**
|
|
- **Finding:** Not applicable. This is not a networked service.
|
|
|
|
### User-Facing Errors
|
|
- [x] **Helpful without exposing internals?**
|
|
- **Finding:** Yes. The error messages are clear and actionable for developers (e.g., "inconsistent dimensionality in points") without revealing sensitive internal state.
|
|
|
|
- [x] **Consistent error format?**
|
|
- **Finding:** Yes. The Go API uses the standard `error` interface. The WASM API has been updated to use a consistent JSON structure for all errors: `{ "ok": false, "error": { "code": "...", "message": "..." } }`.
|
|
|
|
- [ ] **Localization support?**
|
|
- **Finding:** No. Error messages are in English. For a developer-facing library, this is generally acceptable and localization is not expected.
|
|
|
|
### API Errors
|
|
- [x] **Standard error response format?**
|
|
- **Finding:** Yes. As noted above, the WASM API now has a standardized JSON error format.
|
|
|
|
- [ ] **Appropriate HTTP status codes?**
|
|
- **Finding:** Not applicable. This is a WASM module, not an HTTP service.
|
|
|
|
- [x] **Error codes for clients?**
|
|
- **Finding:** Yes. The WASM API now includes standardized string-based error codes (e.g., `bad_request`, `not_found`), allowing clients to handle different error types programmatically.
|
|
|
|
## Logging
|
|
|
|
The library itself does not perform any logging, which is appropriate for its role. It returns errors to the calling application, which is then responsible for its own logging strategy. The library does include analytics and metrics collection (`kdtree_analytics.go`), but this is separate from logging and does not record any sensitive information.
|
|
|
|
### What is Logged
|
|
- [ ] Security events (auth, access)? - **N/A**
|
|
- [ ] Errors with context? - **N/A**
|
|
- [ ] Performance metrics? - **N/A** (collected, but not logged by the library)
|
|
|
|
### What Should NOT be Logged
|
|
- [ ] Passwords/tokens - **N/A**
|
|
- [ ] PII without consent - **N/A**
|
|
- [ ] Full credit card numbers - **N/A**
|
|
|
|
### Log Quality
|
|
- [ ] Structured logging (JSON)? - **N/A**
|
|
- [ ] Correlation IDs? - **N/A**
|
|
- [ ] Log levels used correctly? - **N/A**
|
|
|
|
### Log Security
|
|
- [ ] Injection-safe? - **N/A**
|
|
- [ ] Tamper-evident? - **N/A**
|
|
- [ ] Retention policy? - **N/A**
|