fix(import): return descriptive error messages and success response

[behdadmansouri]

Summary

Fixes ActivityWatch/activitywatch#394

• api.py: Raise a descriptive exception when importing a bucket that already exists (resolves the # TODO: Check that bucket doesn't already exist comment)
• rest.py: Wrap import logic in try/except so errors surface as HTTP 400 with a human-readable {"message": "..."} body instead of a generic 500; return {"message": "Import successful"} on success instead of null

Test plan

• Import a valid export file → server returns {"message": "Import successful"} with 200
• Import a file with a bucket ID that already exists → server returns {"message": "Bucket '...' already exists. Delete it first..."} with 400
• Import a malformed JSON file → server returns a 400 with the parse error message

Note: This PR pairs with a web UI fix in ActivityWatch/aw-webui that surfaces these messages to the user.

🤖 Implemented with Claude Code — AI assistance disclosed per contribution guidelines.

[greptile-apps]

Greptile Summary

This PR improves the /0/import endpoint by resolving a TODO that left duplicate-bucket imports silently broken, and by replacing a bare 200/null response (and potential 500 on error) with descriptive JSON messages.

• api.py: import_bucket now checks for an existing bucket ID before attempting to create it and raises a descriptive error, replacing the # TODO comment.
• rest.py: The import POST handler wraps all import logic in try/except, returning HTTP 400 with {\"message\": \"...\"} on any failure and {\"message\": \"Import successful\"} on success.

Confidence Score: 3/5

Safe for single-bucket imports; multi-bucket imports can leave the database in a partially-written state when a later bucket triggers the new duplicate check.

The new duplicate-bucket check in import_all's loop has no rollback — if bucket N fails, every bucket before it in the iteration is already committed to the database. The 400 response gives no indication of which buckets landed, leaving users without a clear recovery path. This is a real data-consistency gap on the changed import path.

aw_server/api.py — specifically the import_all / import_bucket interaction when processing multi-bucket exports.

Important Files Changed

Filename	Overview
aw_server/api.py	Adds a pre-check in import_bucket that raises Exception if the bucket already exists, resolving the TODO. The check is correct for the single-bucket case, but import_all has no rollback, so multi-bucket imports can leave the DB partially written if a later bucket triggers the error.
aw_server/rest.py	Wraps the import POST handler in try/except to return HTTP 400 with a JSON message on failure instead of a 500, and returns {"message": "Import successful"} on success. Logic is correct; logger.exception is slightly noisy for expected validation errors.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant REST as rest.py (ImportAllResource.post)
    participant API as api.py (ServerAPI.import_all)
    participant DB as Database

    Client->>REST: POST /0/import (file or JSON body)
    REST->>REST: try block entered
    alt File upload
        REST->>REST: parse JSON from file stream
    else JSON body
        REST->>REST: request.get_json()["buckets"]
    end
    REST->>API: import_all(buckets)
    loop For each bucket
        API->>DB: db.buckets() — check existence
        alt Bucket already exists
            API-->>REST: raise Exception("Bucket '...' already exists")
            REST-->>Client: "400 {"message": "Bucket '...' already exists..."}"
        else New bucket
            API->>DB: create_bucket(...)
            API->>DB: create_events(...)
        end
    end
    REST-->>Client: "200 {"message": "Import successful"}"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant REST as rest.py (ImportAllResource.post)
    participant API as api.py (ServerAPI.import_all)
    participant DB as Database

    Client->>REST: POST /0/import (file or JSON body)
    REST->>REST: try block entered
    alt File upload
        REST->>REST: parse JSON from file stream
    else JSON body
        REST->>REST: request.get_json()["buckets"]
    end
    REST->>API: import_all(buckets)
    loop For each bucket
        API->>DB: db.buckets() — check existence
        alt Bucket already exists
            API-->>REST: raise Exception("Bucket '...' already exists")
            REST-->>Client: "400 {"message": "Bucket '...' already exists..."}"
        else New bucket
            API->>DB: create_bucket(...)
            API->>DB: create_events(...)
        end
    end
    REST-->>Client: "200 {"message": "Import successful"}"

Loading

Comments Outside Diff (1)

1. aw_server/api.py, line 138-140 (link)

   Partial import on multi-bucket exports

   import_all iterates over buckets sequentially with no rollback. If a multi-bucket export is uploaded and bucket N already exists, every bucket before N in the iteration order will have been fully written to the database before the error is raised and caught in rest.py. The 400 response body only shows "Bucket 'N' already exists", giving the caller no indication that earlier buckets from the same import were actually committed. This leaves the database in a partially-imported state with no way for the user to know which buckets landed.

   The new explicit pre-check makes this path more reachable than before (previously it would surface only on a DB-level error), so the gap is now more likely to be hit in practice.

_{Reviews (1): Last reviewed commit: "fix(import): return descriptive error me..." | Re-trigger Greptile}

[greptile-apps]

Raising a bare Exception means the caller cannot distinguish this user-facing validation error from unexpected runtime errors. A more specific type (e.g., ValueError) lets rest.py selectively catch expected errors and log them at WARNING level rather than ERROR, while letting unexpected exceptions propagate differently if needed in the future.

Suggested change

	if bucket_id in self.db.buckets():
	raise Exception(
	f"Bucket '{bucket_id}' already exists. Delete it first or rename the bucket before importing."
	)
	if bucket_id in self.db.buckets():
	raise ValueError(
	f"Bucket '{bucket_id}' already exists. Delete it first or rename the bucket before importing."
	)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

logger.exception emits a full traceback. For the expected "bucket already exists" case this is noise — it's a user-facing validation error, not an unexpected server error. Logging it at WARNING (without the traceback) avoids polluting the log with stack traces for routine user mistakes, while keeping real surprises at ERROR/EXCEPTION level.

Suggested change

	except Exception as e:
	logger.exception("Import failed")
	return {"message": str(e)}, 400
	except Exception as e:
	logger.warning("Import failed: %s", e)
	return {"message": str(e)}, 400

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!